On Sat, Nov 16, 2013 at 02:17:51PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> > From an ABI perspective, grub_uboot_syscall is a veneer:
> > the only visible side effect it is permitted to have is to corrupt r12.
> > 
> According to wikipedia:
> "Subroutines must preserve the contents of r4 to r11 and the stack pointer."
> So changing r9 sounds to me like this is actually U-Boot bug and
> preserving it sounds like right way to handle it.

No. grub_uboot_syscall is not a subroutine - it is a veneer.
We have a specific reason to need to preserve r8, over and above what
the ABI says, because u-boot has hijacked it.

But now I went to look at the u-boot code, and suddenly I want to cry.
Commit fe1378a961e508b31b1f29a2bb08ba1dac063155 changes the register
reserved for global data from r8 to r9. Which means we need to preserve
both since they didn't step the API version number.

*sigh*

Updated patch attached.

/
    Leif
>From d1926ea10b47442b4302e0d474cf18dbe65167de Mon Sep 17 00:00:00 2001
From: Leif Lindholm <leif.lindh...@linaro.org>
Date: Sat, 16 Nov 2013 12:15:53 +0000
Subject: [PATCH] arm: fix u-boot port syscall interface va_arg handling

Commit c9cd02c broke the u-boot syscall API for va_args that spill over
to the stack, causing the disk support to stop working. This patch
resolves the problem, while keeping the new, cleaner transition_space
handling.
---
 ChangeLog                          |  5 +++++
 grub-core/kern/arm/uboot/startup.S | 11 ++---------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6d502f8..91c0534 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2013-11-16  Leif Lindholm <leif.lindh...@linaro.org>
+
+	* grub-core/kern/arm/uboot/startup.S: fix grub_uboot_syscall va_arg
+	handling
+
 2013-11-15  Vladimir Serbinenko  <phco...@gmail.com>
 
 	Replace libgcc version of ctz with our own.
diff --git a/grub-core/kern/arm/uboot/startup.S b/grub-core/kern/arm/uboot/startup.S
index f54b14b..df1e329 100644
--- a/grub-core/kern/arm/uboot/startup.S
+++ b/grub-core/kern/arm/uboot/startup.S
@@ -131,11 +131,6 @@ FUNCTION(grub_uboot_syscall)
 	str     r8, transition_space
 	str     lr, transition_space + 4
 	str     r9, transition_space + 8
-	str     sp, transition_space + 12
-
-	sub     sp, sp, #0x20
-	lsr     sp, sp, #3
-	lsl     sp, sp, #3
 
 	ldr	r8, gd_backup
 	ldr	r9, gd_backup + 4
@@ -147,7 +142,6 @@ FUNCTION(grub_uboot_syscall)
 	ldr     r8, transition_space
 	ldr     lr, transition_space + 4
 	ldr     r9, transition_space + 8
-	ldr     sp, transition_space + 12
 
 	bx	lr
 	
@@ -166,8 +160,8 @@ entry_state_end:
 	.long	0	@ r6
 	.long	0	@ r7
 gd_backup:	
-	.long	0	@ r8 - U-Boot global data pointer
-	.long	0	@ r9
+	.long	0	@ r8 - U-Boot global data pointer up to 2013-09-21
+	.long	0	@ r9 - U-Boot global data pointer 2013-09-21 onwards
 	.long	0	@ r10
 	.long	0	@ r11
 VARIABLE(grub_uboot_search_hint)@ U-Boot stack pointer - 
@@ -180,7 +174,6 @@ transition_space:
 	.long	0	@ r8
 	.long	0	@ lr
 	.long	0	@ r9
-	.long	0	@ sp
 
 VARIABLE(grub_uboot_syscall_ptr)
 	.long	0	@
-- 
1.8.4.rc3

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to