Re: [U-Boot] [PATCH] Save/restore global data pointer on API boundary
On 08/07/12 19:30, Wolfgang Denk wrote:
Most architectures keep the global data pointer (gd) in a register.
This may, or may not be. You should not make any assumptions on how
gd is implemented.
The comment did, the code didn't, as long as gd was a pointer (which
should be a safe assumption). But that's irrelevant as I seem to have
gotten the wrong end of the stick.
When using the external app API, because they are calling us rather
than we calling them, this register can be corrupted.
How can this be? The caller should always use the same register
convention as we do - otherwise we are in a much deeper trouble.
It is up to the caller to make sure it uses the published API (resp.
ABI).
Hmm, ok - this was not clear to me from the docs and example.
Indeed, the example does not reserve r8 (on ARM) or r2 (on PPC). Nor
does it save/restore it on entry/syscall. I don't know the exact ABI
semantics of r2 for PPC, but on ARM this is an error.
This could be worked around by something like:
diff --git a/examples/api/crt0.S b/examples/api/crt0.S
index 6daf127..5f956e4 100644
--- a/examples/api/crt0.S
+++ b/examples/api/crt0.S
@@ -49,13 +49,21 @@ syscall:
_start:
ldr ip, =search_hint
str sp, [ip]
+ ldr ip, =gd_backup
+ str r8, [ip]
b main
.globl syscall
syscall:
+ push{r6-r8, lr}
+ ldr r6, =gd_backup
+ ldr r8, [r6]
ldr ip, =syscall_ptr
+ mov lr, pc
ldr pc, [ip]
+ str r8, [r6]
+ pop {r6-r8, pc}
#else
#error No support for this arch!
@@ -69,3 +77,7 @@ syscall_ptr:
.globl search_hint
search_hint:
.long 0
+
+ .globl gd_backup
+gd_backup:
+ .long 0
An alternative would be to mandate DECLARE_GLOBAL_DATA_PTR for all API
applications, but I'd frankly prefer not providing direct access to gd.
Best Regards,
Leif
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] Save/restore global data pointer on API boundary
Most architectures keep the global data pointer (gd) in a register.
When using the external app API, because they are calling us rather
than we calling them, this register can be corrupted.
The attached (trivial) patch saves the gd pointer at api_init(),
and restores it on every entry to syscall(). This may want to be
put behind an ifdef for those architectures that don't use a
dedicated register.
Signed-off-by: Leif Lindholm leif.lindh...@arm.com
---
diff --git a/api/api.c b/api/api.c
index a3bf60a..b911270 100644
--- a/api/api.c
+++ b/api/api.c
@@ -33,6 +33,8 @@
#include api_private.h
+DECLARE_GLOBAL_DATA_PTR;
+
#define DEBUG
#undef DEBUG
@@ -600,6 +602,13 @@ static int API_display_clear(va_list ap)
static cfp_t calls_table[API_MAXCALL] = { NULL, };
/*
+ * The global data pointer is held in a register on most if not all
+ * architectures. Its value is not retained across the API boundary,
+ * so must be manually restored.
+ */
+static void volatile *gd_backup;
+
+/*
* The main syscall entry point - this is not reentrant, only one call is
* serviced until finished.
*
@@ -620,6 +629,8 @@ int syscall(int call, int *retval, ...)
va_list ap;
int rv;
+ gd = gd_backup;
+
if (call 0 || call = calls_no) {
debugf(invalid call #%d\n, call);
return 0;
@@ -686,6 +697,7 @@ void api_init(void)
sig-checksum = crc32(0, (unsigned char *)sig,
sizeof(struct api_signature));
debugf(syscall entry: 0x%08x\n, sig-syscall);
+ gd_backup = gd;
}
void platform_set_mr(struct sys_info *si, unsigned long start, unsigned long
size,
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Save/restore global data pointer on API boundary
Dear Leif Lindholm, In message 50214a38.3000...@arm.com you wrote: Most architectures keep the global data pointer (gd) in a register. This may, or may not be. You should not make any assumptions on how gd is implemented. When using the external app API, because they are calling us rather than we calling them, this register can be corrupted. How can this be? The caller should always use the same register convention as we do - otherwise we are in a much deeper trouble. It is up to the caller to make sure it uses the published API (resp. ABI). The attached (trivial) patch saves the gd pointer at api_init(), and restores it on every entry to syscall(). This may want to be put behind an ifdef for those architectures that don't use a dedicated register. This is wrong. You make assumptions here why may be correct, but may be wrong as well. If we were to implement this, you would have to make sure it always works, no matter how gd is implemented on a specific board / platform. But I doubt this is the right approach. This should be done by the caller. BTW: Please also note that there are a few coding style errors. Checkpatch says: WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt WARNING: please, no spaces at the start of a line (2 x) Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de If the odds are a million to one against something occuring, chances are 50-50 it will. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
