Re: ARM: Clear icache when creating a closure
On 07/25/2011 10:33 AM, Andrew Haley wrote:
> On 21/07/11 16:33, Joseph S. Myers wrote:
>> My suggestion would be putting the instruction sequence in a .s file,
>> rather than hardcoding the instruction encodings here, and writing the
>> code to read from the sequence as assembled by the assembler. That way it
>> will have the appropriate mapping symbols to mark it as ARM-mode code and
>> the linker will deal with adjusting endianness, so you don't need to test
>> for BE-8 at all.
I did this, and it passes the testsuite. Is it OK?
Andrew.
2011-07-26 Andrew Haley
* src/arm/ffi.c (FFI_INIT_TRAMPOLINE): Remove hard-coded assembly
instructions.
* src/arm/sysv.S (ffi_arm_trampoline): Put them here instead.
Index: src/arm/ffi.c
===
--- src/arm/ffi.c (revision 176744)
+++ src/arm/ffi.c (working copy)
@@ -337,14 +337,14 @@
/* How to make a trampoline. */
+extern unsigned int ffi_arm_trampoline[3];
+
#define FFI_INIT_TRAMPOLINE(TRAMP,FUN,CTX) \
({ unsigned char *__tramp = (unsigned char*)(TRAMP); \
unsigned int __fun = (unsigned int)(FUN); \
unsigned int __ctx = (unsigned int)(CTX); \
unsigned char *insns = (unsigned char *)(CTX); \
- *(unsigned int*) &__tramp[0] = 0xe92d000f; /* stmfd sp!, {r0-r3} */ \
- *(unsigned int*) &__tramp[4] = 0xe59f; /* ldr r0, [pc] */ \
- *(unsigned int*) &__tramp[8] = 0xe59ff000; /* ldr pc, [pc] */ \
+ memcpy (__tramp, ffi_arm_trampoline, sizeof ffi_arm_trampoline); \
*(unsigned int*) &__tramp[12] = __ctx; \
*(unsigned int*) &__tramp[16] = __fun; \
__clear_cache((&__tramp[0]), (&__tramp[19])); /* Clear data mapping. */ \
Index: src/arm/sysv.S
===
--- src/arm/sysv.S (revision 176744)
+++ src/arm/sysv.S (working copy)
@@ -461,6 +461,11 @@
UNWIND .fnend
.size
CNAME(ffi_closure_VFP),.ffi_closure_VFP_end-CNAME(ffi_closure_VFP)
+ENTRY(ffi_arm_trampoline)
+ stmfd sp!, {r0-r3}
+ ldr r0, [pc]
+ ldr pc, [pc]
+
#if defined __ELF__ && defined __linux__
.section.note.GNU-stack,"",%progbits
#endif
Re: ARM: Clear icache when creating a closure
On 21/07/11 16:33, Joseph S. Myers wrote:
> On Tue, 12 Jul 2011, Andrew Haley wrote:
>
*(unsigned int*) &__tramp[0] = 0xe92d000f; /* stmfd sp!, {r0-r3} */ \
*(unsigned int*) &__tramp[4] = 0xe59f; /* ldr r0, [pc] */ \
*(unsigned int*) &__tramp[8] = 0xe59ff000; /* ldr pc, [pc] */ \
>
>>> Your patch looks sane, but I'll observe here that the poking of
>>> instruction values is wrong on cores that run in BE-8 mode (where
>>> instructions are always little-endian).
>>
>> Oh dear. How would one test for BE-8 mode on a Linux system?
>
> My suggestion would be putting the instruction sequence in a .s file,
> rather than hardcoding the instruction encodings here, and writing the
> code to read from the sequence as assembled by the assembler. That way it
> will have the appropriate mapping symbols to mark it as ARM-mode code and
> the linker will deal with adjusting endianness, so you don't need to test
> for BE-8 at all.
OK, I'll have a look at doing that.
Andrew.
Re: ARM: Clear icache when creating a closure
On Tue, 12 Jul 2011, Andrew Haley wrote:
> >> *(unsigned int*) &__tramp[0] = 0xe92d000f; /* stmfd sp!, {r0-r3} */ \
> >> *(unsigned int*) &__tramp[4] = 0xe59f; /* ldr r0, [pc] */ \
> >> *(unsigned int*) &__tramp[8] = 0xe59ff000; /* ldr pc, [pc] */ \
> > Your patch looks sane, but I'll observe here that the poking of
> > instruction values is wrong on cores that run in BE-8 mode (where
> > instructions are always little-endian).
>
> Oh dear. How would one test for BE-8 mode on a Linux system?
My suggestion would be putting the instruction sequence in a .s file,
rather than hardcoding the instruction encodings here, and writing the
code to read from the sequence as assembled by the assembler. That way it
will have the appropriate mapping symbols to mark it as ARM-mode code and
the linker will deal with adjusting endianness, so you don't need to test
for BE-8 at all.
--
Joseph S. Myers
jos...@codesourcery.com
Re: ARM: Clear icache when creating a closure
On 12/07/11 10:15, Andrew Haley wrote:
> On 12/07/11 10:12, Richard Earnshaw wrote:
>> On 11/07/11 17:23, Andrew Haley wrote:
>>> On a multicore ARM, you really do have to clear both caches, not just the
>>> dcache. This bug may exist in other ports too.
>>>
>>> Andrew.
>>>
>>>
>>> 2011-07-11 Andrew Haley
>>>
>>> * src/arm/ffi.c (FFI_INIT_TRAMPOLINE): Clear icache.
>>>
>>> diff --git a/src/arm/ffi.c b/src/arm/ffi.c
>>> index 885a9cb..b2e7667 100644
>>> --- a/src/arm/ffi.c
>>> +++ b/src/arm/ffi.c
>>> @@ -558,12 +558,16 @@ ffi_closure_free (void *ptr)
>>> ({ unsigned char *__tramp = (unsigned char*)(TRAMP); \
>>> unsigned int __fun = (unsigned int)(FUN); \
>>> unsigned int __ctx = (unsigned int)(CTX); \
>>> + unsigned char *insns = (unsigned char *)(CTX); \
>>> *(unsigned int*) &__tramp[0] = 0xe92d000f; /* stmfd sp!, {r0-r3} */ \
>>> *(unsigned int*) &__tramp[4] = 0xe59f; /* ldr r0, [pc] */ \
>>> *(unsigned int*) &__tramp[8] = 0xe59ff000; /* ldr pc, [pc] */ \
>>> *(unsigned int*) &__tramp[12] = __ctx; \
>>> *(unsigned int*) &__tramp[16] = __fun; \
>>> - __clear_cache((&__tramp[0]), (&__tramp[19])); \
>>> + __clear_cache((&__tramp[0]), (&__tramp[19])); /* Clear data mapping.
>>> */ \
>>> + __clear_cache(insns, insns + 3 * sizeof (unsigned int)); \
>>> + /* Clear instruction \
>>> +mapping. */\
>>> })
>>>
>>> #endif
>>>
>>>
>>
>>
>> Your patch looks sane, but I'll observe here that the poking of
>> instruction values is wrong on cores that run in BE-8 mode (where
>> instructions are always little-endian).
>
> Oh dear. How would one test for BE-8 mode on a Linux system?
>
> Thanks,
> Andrew.
>
>
Essentially v6 or later and big-endian. It is possible to run some v6
(but no v7) cores in be-32 mode, but you can't then have unaligned
access support.
To know the configuration for sure, you need to read the SCTLR register
(in CP15 space), but that's not available in user-mode.
R.
Re: ARM: Clear icache when creating a closure
On 12/07/11 10:12, Richard Earnshaw wrote:
> On 11/07/11 17:23, Andrew Haley wrote:
>> On a multicore ARM, you really do have to clear both caches, not just the
>> dcache. This bug may exist in other ports too.
>>
>> Andrew.
>>
>>
>> 2011-07-11 Andrew Haley
>>
>> * src/arm/ffi.c (FFI_INIT_TRAMPOLINE): Clear icache.
>>
>> diff --git a/src/arm/ffi.c b/src/arm/ffi.c
>> index 885a9cb..b2e7667 100644
>> --- a/src/arm/ffi.c
>> +++ b/src/arm/ffi.c
>> @@ -558,12 +558,16 @@ ffi_closure_free (void *ptr)
>> ({ unsigned char *__tramp = (unsigned char*)(TRAMP); \
>> unsigned int __fun = (unsigned int)(FUN); \
>> unsigned int __ctx = (unsigned int)(CTX); \
>> + unsigned char *insns = (unsigned char *)(CTX); \
>> *(unsigned int*) &__tramp[0] = 0xe92d000f; /* stmfd sp!, {r0-r3} */ \
>> *(unsigned int*) &__tramp[4] = 0xe59f; /* ldr r0, [pc] */ \
>> *(unsigned int*) &__tramp[8] = 0xe59ff000; /* ldr pc, [pc] */ \
>> *(unsigned int*) &__tramp[12] = __ctx; \
>> *(unsigned int*) &__tramp[16] = __fun; \
>> - __clear_cache((&__tramp[0]), (&__tramp[19])); \
>> + __clear_cache((&__tramp[0]), (&__tramp[19])); /* Clear data mapping. */
>> \
>> + __clear_cache(insns, insns + 3 * sizeof (unsigned int)); \
>> + /* Clear instruction \
>> +mapping. */\
>> })
>>
>> #endif
>>
>>
>
>
> Your patch looks sane, but I'll observe here that the poking of
> instruction values is wrong on cores that run in BE-8 mode (where
> instructions are always little-endian).
Oh dear. How would one test for BE-8 mode on a Linux system?
Thanks,
Andrew.
Re: ARM: Clear icache when creating a closure
On 11/07/11 17:23, Andrew Haley wrote:
> On a multicore ARM, you really do have to clear both caches, not just the
> dcache. This bug may exist in other ports too.
>
> Andrew.
>
>
> 2011-07-11 Andrew Haley
>
> * src/arm/ffi.c (FFI_INIT_TRAMPOLINE): Clear icache.
>
> diff --git a/src/arm/ffi.c b/src/arm/ffi.c
> index 885a9cb..b2e7667 100644
> --- a/src/arm/ffi.c
> +++ b/src/arm/ffi.c
> @@ -558,12 +558,16 @@ ffi_closure_free (void *ptr)
> ({ unsigned char *__tramp = (unsigned char*)(TRAMP); \
> unsigned int __fun = (unsigned int)(FUN); \
> unsigned int __ctx = (unsigned int)(CTX); \
> + unsigned char *insns = (unsigned char *)(CTX); \
> *(unsigned int*) &__tramp[0] = 0xe92d000f; /* stmfd sp!, {r0-r3} */ \
> *(unsigned int*) &__tramp[4] = 0xe59f; /* ldr r0, [pc] */ \
> *(unsigned int*) &__tramp[8] = 0xe59ff000; /* ldr pc, [pc] */ \
> *(unsigned int*) &__tramp[12] = __ctx; \
> *(unsigned int*) &__tramp[16] = __fun; \
> - __clear_cache((&__tramp[0]), (&__tramp[19])); \
> + __clear_cache((&__tramp[0]), (&__tramp[19])); /* Clear data mapping. */ \
> + __clear_cache(insns, insns + 3 * sizeof (unsigned int)); \
> + /* Clear instruction \
> +mapping. */\
> })
>
> #endif
>
>
Your patch looks sane, but I'll observe here that the poking of
instruction values is wrong on cores that run in BE-8 mode (where
instructions are always little-endian).
R.
ARM: Clear icache when creating a closure
On a multicore ARM, you really do have to clear both caches, not just the
dcache. This bug may exist in other ports too.
Andrew.
2011-07-11 Andrew Haley
* src/arm/ffi.c (FFI_INIT_TRAMPOLINE): Clear icache.
diff --git a/src/arm/ffi.c b/src/arm/ffi.c
index 885a9cb..b2e7667 100644
--- a/src/arm/ffi.c
+++ b/src/arm/ffi.c
@@ -558,12 +558,16 @@ ffi_closure_free (void *ptr)
({ unsigned char *__tramp = (unsigned char*)(TRAMP); \
unsigned int __fun = (unsigned int)(FUN); \
unsigned int __ctx = (unsigned int)(CTX); \
+ unsigned char *insns = (unsigned char *)(CTX); \
*(unsigned int*) &__tramp[0] = 0xe92d000f; /* stmfd sp!, {r0-r3} */ \
*(unsigned int*) &__tramp[4] = 0xe59f; /* ldr r0, [pc] */ \
*(unsigned int*) &__tramp[8] = 0xe59ff000; /* ldr pc, [pc] */ \
*(unsigned int*) &__tramp[12] = __ctx; \
*(unsigned int*) &__tramp[16] = __fun; \
- __clear_cache((&__tramp[0]), (&__tramp[19])); \
+ __clear_cache((&__tramp[0]), (&__tramp[19])); /* Clear data mapping. */ \
+ __clear_cache(insns, insns + 3 * sizeof (unsigned int)); \
+ /* Clear instruction \
+mapping. */\
})
#endif
