[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Dave Martin]

In low-level board support code, there is sometimes a need to
copy a function body to another location at run-time.

A straightforward call to memcpy doesn't work in Thumb-2,
because bit 0 of external Thumb function symbols is set to 1,
indicating that the function is Thumb.  Without corrective
measures, this will cause an off-by-one copy, and the copy
may be called using the wrong instruction set.

This patch adds an fncpy() macro to help with such copies.

Particular care is needed, because C doesn't guarantee any
defined behaviour when casting a function pointer to any other
type.  This has been observed to lead to strange optimisation
side-effects when doing the arithmetic which is required in
order to copy/move function bodies correctly in Thumb-2.

Thanks to Russell King and Nicolas Pitre for their input
on this patch.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/include/asm/fncpy.h |   96 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 96 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/fncpy.h

diff --git a/arch/arm/include/asm/fncpy.h b/arch/arm/include/asm/fncpy.h
new file mode 100644
index 0000000..8b94b5f
--- /dev/null
+++ b/arch/arm/include/asm/fncpy.h
@@ -0,0 +1,96 @@
+/*
+ * arch/arm/include/asm/fncpy.h - helper macros for function body copying
+ *
+ * Copyright (C) 2011 Linaro Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+/*
+ * These macros are intended for use when there is a need to copy a low-level
+ * function body into special memory.
+ *
+ * For example, when reconfiguring the SDRAM controller, the code doing the
+ * reconfiguration may need to run from SRAM.
+ *
+ * NOTE: that the copied function body must be entirely self-contained and
+ * position-independent in order for this to work properly.
+ *
+ * NOTE: in order for embedded literals and data to get referenced correctly,
+ * the alignment of functions must be preserved when copying.  To ensure this,
+ * the source and destination addresses for fncpy() must be aligned to a
+ * multiple of 8 bytes: you will be get a BUG() if this condition is not met.
+ * You will typically need a ".align 3" directive in the assembler where the
+ * function to be copied is defined, and ensure that your allocator for the
+ * destination buffer returns 8-byte-aligned pointers.
+ *
+ * Typical usage example:
+ *
+ * extern int f(args);
+ * extern uint32_t size_of_f;
+ * int (*copied_f)(args);
+ * void *sram_buffer;
+ *
+ * copied_f = fncpy(sram_buffer, &f, size_of_f);
+ *
+ * ... do any required D-side/I-side synchronisation ...
+ *
+ * ... later, call the function: ...
+ *
+ * copied_f(args);
+ *
+ * The size of the function to be copied can't be determined from C:
+ * this must be determined by other means, such as adding assmbler directives
+ * in the file where f is defined.
+ */
+
+#ifndef __ASM_FNCPY_H
+#define __ASM_FNCPY_H
+
+#include <linux/types.h>
+#include <linux/string.h>
+
+#include <asm/bug.h>
+#include <asm/cacheflush.h>
+
+/*
+ * Minimum alignment requirement for the source and destination addresses
+ * for function copying.
+ */
+#define FNCPY_ALIGN 8
+
+#define fncpy(dest_buf, funcp, size) ({					\
+	uintptr_t __funcp_address;					\
+	typeof(funcp) __result;						\
+									\
+	asm("" : "=r" (__funcp_address) : "0" (funcp));			\
+									\
+	/*								\
+	 * Ensure alignment of source and destination addresses,	\
+	 * disregarding the function's Thumb bit:			\
+	 */								\
+	BUG_ON((uintptr_t)(dest_buf) & (FNCPY_ALIGN - 1) ||		\
+		(__funcp_address & ~(uintptr_t)1 & (FNCPY_ALIGN - 1)));	\
+									\
+	memcpy(dest_buf, (void const *)(__funcp_address & ~1), size);	\
+	flush_icache_range((unsigned long)(dest_buf),			\
+		(unsigned long)(dest_buf) + (size));			\
+									\
+	asm("" : "=r" (__result)					\
+		: "0" ((uintptr_t)(dest_buf) | (__funcp_address & 1)));	\
+									\
+	__result;							\
+})
+
+#endif /* !__ASM_FNCPY_H */
-- 
1.7.1

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Jean Pihet]

On Fri, Jan 14, 2011 at 10:17 PM, Dave Martin <dave.martin@linaro.org> wrote:
> In low-level board support code, there is sometimes a need to
> copy a function body to another location at run-time.
>
> A straightforward call to memcpy doesn't work in Thumb-2,
> because bit 0 of external Thumb function symbols is set to 1,
> indicating that the function is Thumb. ?Without corrective
> measures, this will cause an off-by-one copy, and the copy
> may be called using the wrong instruction set.
>
> This patch adds an fncpy() macro to help with such copies.
>
> Particular care is needed, because C doesn't guarantee any
> defined behaviour when casting a function pointer to any other
> type. ?This has been observed to lead to strange optimisation
> side-effects when doing the arithmetic which is required in
> order to copy/move function bodies correctly in Thumb-2.
>
> Thanks to Russell King and Nicolas Pitre for their input
> on this patch.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> Tested-by: Jean Pihet <j-pihet@ti.com>
> ---
> ?arch/arm/include/asm/fncpy.h | ? 96 ++++++++++++++++++++++++++++++++++++++++++
> ?1 files changed, 96 insertions(+), 0 deletions(-)
> ?create mode 100644 arch/arm/include/asm/fncpy.h
>
> diff --git a/arch/arm/include/asm/fncpy.h b/arch/arm/include/asm/fncpy.h
> new file mode 100644
> index 0000000..8b94b5f
> --- /dev/null
> +++ b/arch/arm/include/asm/fncpy.h
> @@ -0,0 +1,96 @@
> +/*
> + * arch/arm/include/asm/fncpy.h - helper macros for function body copying
> + *
> + * Copyright (C) 2011 Linaro Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +/*
> + * These macros are intended for use when there is a need to copy a low-level
> + * function body into special memory.
> + *
> + * For example, when reconfiguring the SDRAM controller, the code doing the
> + * reconfiguration may need to run from SRAM.
> + *
> + * NOTE: that the copied function body must be entirely self-contained and
> + * position-independent in order for this to work properly.
> + *
> + * NOTE: in order for embedded literals and data to get referenced correctly,
> + * the alignment of functions must be preserved when copying. ?To ensure this,
> + * the source and destination addresses for fncpy() must be aligned to a
> + * multiple of 8 bytes: you will be get a BUG() if this condition is not met.
> + * You will typically need a ".align 3" directive in the assembler where the
> + * function to be copied is defined, and ensure that your allocator for the
> + * destination buffer returns 8-byte-aligned pointers.

Note that aligning the source and destination pointers to a multiple
of 8 bytes has an impact on the behavio(u)r and so must be carefully
thought and tested on OMAP1/2/3 platforms.

Regards,
Jean

> + *
> + * Typical usage example:
> + *
> + * extern int f(args);
> + * extern uint32_t size_of_f;
> + * int (*copied_f)(args);
> + * void *sram_buffer;
> + *
> + * copied_f = fncpy(sram_buffer, &f, size_of_f);
> + *
> + * ... do any required D-side/I-side synchronisation ...
> + *
> + * ... later, call the function: ...
> + *
> + * copied_f(args);
> + *
> + * The size of the function to be copied can't be determined from C:
> + * this must be determined by other means, such as adding assmbler directives
> + * in the file where f is defined.
> + */
> +
> +#ifndef __ASM_FNCPY_H
> +#define __ASM_FNCPY_H
> +
> +#include <linux/types.h>
> +#include <linux/string.h>
> +
> +#include <asm/bug.h>
> +#include <asm/cacheflush.h>
> +
> +/*
> + * Minimum alignment requirement for the source and destination addresses
> + * for function copying.
> + */
> +#define FNCPY_ALIGN 8
> +
> +#define fncpy(dest_buf, funcp, size) ({ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? uintptr_t __funcp_address; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? typeof(funcp) __result; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? asm("" : "=r" (__funcp_address) : "0" (funcp)); ? ? ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? /* ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? ?* Ensure alignment of source and destination addresses, ? ? ? ?\
> + ? ? ? ?* disregarding the function's Thumb bit: ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? ?*/ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? BUG_ON((uintptr_t)(dest_buf) & (FNCPY_ALIGN - 1) || ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? (__funcp_address & ~(uintptr_t)1 & (FNCPY_ALIGN - 1))); \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? memcpy(dest_buf, (void const *)(__funcp_address & ~1), size); ? \
> + ? ? ? flush_icache_range((unsigned long)(dest_buf), ? ? ? ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? (unsigned long)(dest_buf) + (size)); ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? asm("" : "=r" (__result) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? ? ? ? ? : "0" ((uintptr_t)(dest_buf) | (__funcp_address & 1))); \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? __result; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> +})
> +
> +#endif /* !__ASM_FNCPY_H */
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Dave Martin]

Hi,

On Mon, Jan 17, 2011 at 2:02 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote:

[...]

> Note that aligning the source and destination pointers to a multiple
> of 8 bytes has an impact on the behavio(u)r and so must be carefully
> thought and tested on OMAP1/2/3 platforms.

Do you have any specific concerns regarding this?

Currently, the only issue I can think of is that the need to allocate
aligned memory from the SRAM will increase the total amount allocated,
which could be a problem if we end up overflowing the available SRAM.

This does not appear to happen in the case I've tested -- I currently
round up the amount allocated in omap_sram_push to be a multiple of 8
bytes.  This, combined with a couple of ".align 3" directives, is
enough to get me a booting system on omap3... but I haven't tested
exhaustively.

Cheers
---Dave

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Santosh Shilimkar]

> -----Original Message-----
> From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-
> owner at vger.kernel.org] On Behalf Of Dave Martin
> Sent: Monday, January 17, 2011 9:06 PM
> To: Jean Pihet
> Cc: linux-arm-kernel at lists.infradead.org; linux-
> omap at vger.kernel.org; Jean Pihet
> Subject: Re: [PATCH v4] ARM: Thumb-2: Symbol manipulation macros for
> function body copying
>
> Hi,
>
> On Mon, Jan 17, 2011 at 2:02 PM, Jean Pihet
> <jean.pihet@newoldbits.com> wrote:
>
> [...]
>
> > Note that aligning the source and destination pointers to a
> multiple
> > of 8 bytes has an impact on the behavio(u)r and so must be
> carefully
> > thought and tested on OMAP1/2/3 platforms.
>
> Do you have any specific concerns regarding this?
>
> Currently, the only issue I can think of is that the need to
> allocate
> aligned memory from the SRAM will increase the total amount
> allocated,
> which could be a problem if we end up overflowing the available
> SRAM.
>
> This does not appear to happen in the case I've tested -- I
> currently
> round up the amount allocated in omap_sram_push to be a multiple of
> 8
> bytes.  This, combined with a couple of ".align 3" directives, is
> enough to get me a booting system on omap3... but I haven't tested
> exhaustively.
>
I don't think there can be overflow issue considering it's current
use and available SRAM on OMAP.

How much additional memory you will need to take care of
alignment.

Max additional memory = total fns * ( 8 + 8)
			    = ~ 10 * 16
			    = 160 bytes.

Should be ok.

Regards,
Santosh

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Jean Pihet]

Hi Dave,

On Mon, Jan 17, 2011 at 4:35 PM, Dave Martin <dave.martin@linaro.org> wrote:
> Hi,
>
> On Mon, Jan 17, 2011 at 2:02 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
>
> [...]
>
>> Note that aligning the source and destination pointers to a multiple
>> of 8 bytes has an impact on the behavio(u)r and so must be carefully
>> thought and tested on OMAP1/2/3 platforms.
>
> Do you have any specific concerns regarding this?
>
> Currently, the only issue I can think of is that the need to allocate
> aligned memory from the SRAM will increase the total amount allocated,
> which could be a problem if we end up overflowing the available SRAM.
Agree. It does not look like there are SRAM overflows today. Note that
in that case you will get warned soon enough by the 'Not enough space
in SRAM' message.
One could think about some nasty side-effects bugs like badly written
PIC code, hardcoded addresses... that appear to work with the current
version. In short this needs to be thoroughly tested on OMAP1/2/3
platforms.

> This does not appear to happen in the case I've tested -- I currently
> round up the amount allocated in omap_sram_push to be a multiple of 8
> bytes. ?This, combined with a couple of ".align 3" directives, is
> enough to get me a booting system on omap3... but I haven't tested
> exhaustively.
That is OK. I have a patch ready for OMAP1/2/3, tested on OMAP3 only.

> Cheers
> ---Dave
>

Thanks,
Jean

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Russell King - ARM Linux]

On Mon, Jan 17, 2011 at 03:02:20PM +0100, Jean Pihet wrote:
> On Fri, Jan 14, 2011 at 10:17 PM, Dave Martin <dave.martin@linaro.org> wrote:
> > + * These macros are intended for use when there is a need to copy a low-level
> > + * function body into special memory.
> > + *
> > + * For example, when reconfiguring the SDRAM controller, the code doing the
> > + * reconfiguration may need to run from SRAM.
> > + *
> > + * NOTE: that the copied function body must be entirely self-contained and
> > + * position-independent in order for this to work properly.
> > + *
> > + * NOTE: in order for embedded literals and data to get referenced correctly,
> > + * the alignment of functions must be preserved when copying. ?To ensure this,
> > + * the source and destination addresses for fncpy() must be aligned to a
> > + * multiple of 8 bytes: you will be get a BUG() if this condition is not met.
> > + * You will typically need a ".align 3" directive in the assembler where the
> > + * function to be copied is defined, and ensure that your allocator for the
> > + * destination buffer returns 8-byte-aligned pointers.
> 
> Note that aligning the source and destination pointers to a multiple
> of 8 bytes has an impact on the behavio(u)r and so must be carefully
> thought and tested on OMAP1/2/3 platforms.

OMAP3 is ARMv7, so is EABI.  EABI requires 64-bit data to be aligned to
natural 64-bit boundaries, so architecturally it's correct.

Nevertheless, the code may not be using 64-bit data, so that doesn't
apply - but fncpy() can't know that.

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Tony Lindgren]

* Dave Martin <dave.martin@linaro.org> [110114 13:17]:
> In low-level board support code, there is sometimes a need to
> copy a function body to another location at run-time.
> 
> A straightforward call to memcpy doesn't work in Thumb-2,
> because bit 0 of external Thumb function symbols is set to 1,
> indicating that the function is Thumb.  Without corrective
> measures, this will cause an off-by-one copy, and the copy
> may be called using the wrong instruction set.
> 
> This patch adds an fncpy() macro to help with such copies.
> 
> Particular care is needed, because C doesn't guarantee any
> defined behaviour when casting a function pointer to any other
> type.  This has been observed to lead to strange optimisation
> side-effects when doing the arithmetic which is required in
> order to copy/move function bodies correctly in Thumb-2.
> 
> Thanks to Russell King and Nicolas Pitre for their input
> on this patch.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> Tested-by: Jean Pihet <j-pihet@ti.com>

Boot tested on osk5912 and n800:

Tested-by: Tony Lindgren <tony@atomide.com>

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Kevin Hilman]

Dave Martin <dave.martin@linaro.org> writes:

> In low-level board support code, there is sometimes a need to
> copy a function body to another location at run-time.
>
> A straightforward call to memcpy doesn't work in Thumb-2,
> because bit 0 of external Thumb function symbols is set to 1,
> indicating that the function is Thumb.  Without corrective
> measures, this will cause an off-by-one copy, and the copy
> may be called using the wrong instruction set.
>
> This patch adds an fncpy() macro to help with such copies.
>
> Particular care is needed, because C doesn't guarantee any
> defined behaviour when casting a function pointer to any other
> type.  This has been observed to lead to strange optimisation
> side-effects when doing the arithmetic which is required in
> order to copy/move function bodies correctly in Thumb-2.
>
> Thanks to Russell King and Nicolas Pitre for their input
> on this patch.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> Tested-by: Jean Pihet <j-pihet@ti.com>

Tested-by: Kevin Hilman <khilman@ti.com>

along with Jean's OMAP patch on:

OMAP2420/n810: including basic suspend/resume test.

OMAP16xx/OSK: boot test only.

Kevin

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Dave Martin]

On Wed, Jan 19, 2011 at 10:29 PM, Kevin Hilman <khilman@ti.com> wrote:
> Dave Martin <dave.martin@linaro.org> writes:
>
>> In low-level board support code, there is sometimes a need to
>> copy a function body to another location at run-time.
>>
>> A straightforward call to memcpy doesn't work in Thumb-2,
>> because bit 0 of external Thumb function symbols is set to 1,
>> indicating that the function is Thumb. ?Without corrective
>> measures, this will cause an off-by-one copy, and the copy
>> may be called using the wrong instruction set.
>>
>> This patch adds an fncpy() macro to help with such copies.
>>
>> Particular care is needed, because C doesn't guarantee any
>> defined behaviour when casting a function pointer to any other
>> type. ?This has been observed to lead to strange optimisation
>> side-effects when doing the arithmetic which is required in
>> order to copy/move function bodies correctly in Thumb-2.
>>
>> Thanks to Russell King and Nicolas Pitre for their input
>> on this patch.
>>
>> Signed-off-by: Dave Martin <dave.martin@linaro.org>
>> Tested-by: Jean Pihet <j-pihet@ti.com>
>
> Tested-by: Kevin Hilman <khilman@ti.com>
>
> along with Jean's OMAP patch on:
>
> OMAP2420/n810: including basic suspend/resume test.
>
> OMAP16xx/OSK: boot test only.
>
> Kevin
>

Thanks
---Dave

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Dave Martin]

On Thu, Jan 20, 2011 at 9:42 AM, Dave Martin <dave.martin@linaro.org> wrote:
> On Wed, Jan 19, 2011 at 10:29 PM, Kevin Hilman <khilman@ti.com> wrote:
>> Dave Martin <dave.martin@linaro.org> writes:
>>
>>> In low-level board support code, there is sometimes a need to
>>> copy a function body to another location at run-time.
>>>
>>> A straightforward call to memcpy doesn't work in Thumb-2,
>>> because bit 0 of external Thumb function symbols is set to 1,
>>> indicating that the function is Thumb. ?Without corrective
>>> measures, this will cause an off-by-one copy, and the copy
>>> may be called using the wrong instruction set.
>>>
>>> This patch adds an fncpy() macro to help with such copies.
>>>
>>> Particular care is needed, because C doesn't guarantee any
>>> defined behaviour when casting a function pointer to any other
>>> type. ?This has been observed to lead to strange optimisation
>>> side-effects when doing the arithmetic which is required in
>>> order to copy/move function bodies correctly in Thumb-2.
>>>
>>> Thanks to Russell King and Nicolas Pitre for their input
>>> on this patch.
>>>
>>> Signed-off-by: Dave Martin <dave.martin@linaro.org>
>>> Tested-by: Jean Pihet <j-pihet@ti.com>
>>
>> Tested-by: Kevin Hilman <khilman@ti.com>
>>
>> along with Jean's OMAP patch on:
>>
>> OMAP2420/n810: including basic suspend/resume test.
>>
>> OMAP16xx/OSK: boot test only.
>>
>> Kevin
>>
>
> Thanks
> ---Dave
>

Any more comments on this patch?

I have no further changes so far.

Cheers
---Dave

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Jean Pihet]

Hi Dave,

On Mon, Jan 24, 2011 at 2:50 PM, Dave Martin <dave.martin@linaro.org> wrote:
> On Thu, Jan 20, 2011 at 9:42 AM, Dave Martin <dave.martin@linaro.org> wrote:
>> On Wed, Jan 19, 2011 at 10:29 PM, Kevin Hilman <khilman@ti.com> wrote:
>>> Dave Martin <dave.martin@linaro.org> writes:
>>>
>>>> In low-level board support code, there is sometimes a need to
>>>> copy a function body to another location at run-time.
>>>>
>>>> A straightforward call to memcpy doesn't work in Thumb-2,
>>>> because bit 0 of external Thumb function symbols is set to 1,
>>>> indicating that the function is Thumb. ?Without corrective
>>>> measures, this will cause an off-by-one copy, and the copy
>>>> may be called using the wrong instruction set.
>>>>
>>>> This patch adds an fncpy() macro to help with such copies.
>>>>
>>>> Particular care is needed, because C doesn't guarantee any
>>>> defined behaviour when casting a function pointer to any other
>>>> type. ?This has been observed to lead to strange optimisation
>>>> side-effects when doing the arithmetic which is required in
>>>> order to copy/move function bodies correctly in Thumb-2.
>>>>
>>>> Thanks to Russell King and Nicolas Pitre for their input
>>>> on this patch.
>>>>
>>>> Signed-off-by: Dave Martin <dave.martin@linaro.org>
>>>> Tested-by: Jean Pihet <j-pihet@ti.com>
>>>
>>> Tested-by: Kevin Hilman <khilman@ti.com>
>>>
>>> along with Jean's OMAP patch on:
>>>
>>> OMAP2420/n810: including basic suspend/resume test.
>>>
>>> OMAP16xx/OSK: boot test only.
>>>
>>> Kevin
>>>
>>
>> Thanks
>> ---Dave
>>
>
> Any more comments on this patch?
>
> I have no further changes so far.
Ok to me. The changes are now in the omap-testing branch of Tony's tree [1].

[1] http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=shortlog;h=refs/heads/omap-testing

Regards,
Jean

>
> Cheers
> ---Dave
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Dave Martin]

Hi Russell,

I tried to send this patch to your patch system, but I've received no
response or any bounce.  On my end the mail appears to have been sent,
and previous submissions made via the same route have succeeded.

The patch doesn't seem to be in the patch system, but I'm not sure
what went wrong ... hopefully I'm not doing anything stupid.

Do you have any ideas what might have gone wrong?

Cheers
---Dave

---------- Forwarded message ----------
From: Dave Martin <dave.martin@linaro.org>
Date: Wed, Jan 26, 2011 at 11:57 AM
Subject: [PATCH v4] ARM: Thumb-2: Symbol manipulation macros for
function body copying
To: patches at arm.linux.org.uk
Cc: Dave Martin <dave.martin@linaro.org>


In low-level board support code, there is sometimes a need to
copy a function body to another location at run-time.

A straightforward call to memcpy doesn't work in Thumb-2,
because bit 0 of external Thumb function symbols is set to 1,
indicating that the function is Thumb.  Without corrective
measures, this will cause an off-by-one copy, and the copy
may be called using the wrong instruction set.

This patch adds an fncpy() macro to help with such copies.

Particular care is needed, because C doesn't guarantee any
defined behaviour when casting a function pointer to any other
type.  This has been observed to lead to strange optimisation
side-effects when doing the arithmetic which is required in
order to copy/move function bodies correctly in Thumb-2.

Thanks to Russell King and Nicolas Pitre for their input
on this patch.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Jean Pihet <j-pihet@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Kevin Hilman <khilman@ti.com>
--
KernelVersion: 2.6.37

diff --git a/arch/arm/include/asm/fncpy.h b/arch/arm/include/asm/fncpy.h
new file mode 100644
index 0000000..8b94b5f
--- /dev/null
+++ b/arch/arm/include/asm/fncpy.h
@@ -0,0 +1,96 @@
+/*
+ * arch/arm/include/asm/fncpy.h - helper macros for function body copying
+ *
+ * Copyright (C) 2011 Linaro Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+/*
+ * These macros are intended for use when there is a need to copy a low-level
+ * function body into special memory.
+ *
+ * For example, when reconfiguring the SDRAM controller, the code doing the
+ * reconfiguration may need to run from SRAM.
+ *
+ * NOTE: that the copied function body must be entirely self-contained and
+ * position-independent in order for this to work properly.
+ *
+ * NOTE: in order for embedded literals and data to get referenced correctly,
+ * the alignment of functions must be preserved when copying.  To ensure this,
+ * the source and destination addresses for fncpy() must be aligned to a
+ * multiple of 8 bytes: you will be get a BUG() if this condition is not met.
+ * You will typically need a ".align 3" directive in the assembler where the
+ * function to be copied is defined, and ensure that your allocator for the
+ * destination buffer returns 8-byte-aligned pointers.
+ *
+ * Typical usage example:
+ *
+ * extern int f(args);
+ * extern uint32_t size_of_f;
+ * int (*copied_f)(args);
+ * void *sram_buffer;
+ *
+ * copied_f = fncpy(sram_buffer, &f, size_of_f);
+ *
+ * ... later, call the function: ...
+ *
+ * copied_f(args);
+ *
+ * The size of the function to be copied can't be determined from C:
+ * this must be determined by other means, such as adding assmbler directives
+ * in the file where f is defined.
+ */
+
+#ifndef __ASM_FNCPY_H
+#define __ASM_FNCPY_H
+
+#include <linux/types.h>
+#include <linux/string.h>
+
+#include <asm/bug.h>
+#include <asm/cacheflush.h>
+
+/*
+ * Minimum alignment requirement for the source and destination addresses
+ * for function copying.
+ */
+#define FNCPY_ALIGN 8
+
+#define fncpy(dest_buf, funcp, size) ({
         \
+       uintptr_t __funcp_address;                                      \
+       typeof(funcp) __result;                                         \
+                                                                       \
+       asm("" : "=r" (__funcp_address) : "0" (funcp));                 \
+                                                                       \
+       /*                                                              \
+        * Ensure alignment of source and destination addresses,        \
+        * disregarding the function's Thumb bit:                       \
+        */                                                             \
+       BUG_ON((uintptr_t)(dest_buf) & (FNCPY_ALIGN - 1) ||             \
+               (__funcp_address & ~(uintptr_t)1 & (FNCPY_ALIGN - 1))); \
+                                                                       \
+       memcpy(dest_buf, (void const *)(__funcp_address & ~1), size);   \
+       flush_icache_range((unsigned long)(dest_buf),                   \
+               (unsigned long)(dest_buf) + (size));                    \
+                                                                       \
+       asm("" : "=r" (__result)                                        \
+               : "0" ((uintptr_t)(dest_buf) | (__funcp_address & 1))); \
+                                                                       \
+       __result;                                                       \
+})
+
+#endif /* !__ASM_FNCPY_H */
--
1.7.1

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Russell King - ARM Linux]

On Wed, Jan 26, 2011 at 04:05:33PM +0000, Dave Martin wrote:
> Hi Russell,
> 
> I tried to send this patch to your patch system, but I've received no
> response or any bounce.  On my end the mail appears to have been sent,
> and previous submissions made via the same route have succeeded.
> 
> The patch doesn't seem to be in the patch system, but I'm not sure
> what went wrong ... hopefully I'm not doing anything stupid.
> 
> Do you have any ideas what might have gone wrong?

Probably the lack of a line matching '^PATCH\s+FOLLOWS\s*$', '^--[- ]$'.

It's rather particular about that before it'll even consider sending a
reply,  It's a measure to prevent excessive collateral spam and my MTA
being blacklisted as a result.

[PATCH v4] ARM: Thumb-2: Symbol manipulation macros for function body copying

[Dave Martin]

On Wed, Jan 26, 2011 at 4:38 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jan 26, 2011 at 04:05:33PM +0000, Dave Martin wrote:
>> Hi Russell,
>>
>> I tried to send this patch to your patch system, but I've received no
>> response or any bounce. ?On my end the mail appears to have been sent,
>> and previous submissions made via the same route have succeeded.
>>
>> The patch doesn't seem to be in the patch system, but I'm not sure
>> what went wrong ... hopefully I'm not doing anything stupid.
>>
>> Do you have any ideas what might have gone wrong?
>
> Probably the lack of a line matching '^PATCH\s+FOLLOWS\s*$', '^--[- ]$'.
>
> It's rather particular about that before it'll even consider sending a
> reply,  It's a measure to prevent excessive collateral spam and my MTA
> being blacklisted as a result.
>

Darn, not enough ---

Looks like I did something stupid after all...

The patch should be successfully posted now -- apologies for the spam!

Cheers
---Dave