[PATCH 0/3] Generalize fncpy availability

[PATCH 0/3] Generalize fncpy availability

[Florian Fainelli]

Hi all,

This patch series relocates ARM's fncpy to asm-generic in order for us to
be able to use SRAM_EXEC on ARM64 platforms.

Florian Fainelli (3):
  ARM: Generalize fncpy implementation
  arm64: Provide a fncpy implenentation
  misc: sram: Allow ARM64 to select SRAM_EXEC

 arch/arm/include/asm/fncpy.h   | 77 ++--------------------------------
 arch/arm64/include/asm/fncpy.h |  6 +++
 drivers/misc/Kconfig           |  2 +-
 include/asm-generic/fncpy.h    | 94 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 104 insertions(+), 75 deletions(-)
 create mode 100644 arch/arm64/include/asm/fncpy.h
 create mode 100644 include/asm-generic/fncpy.h

-- 
2.9.3

[PATCH 1/3] ARM: Generalize fncpy implementation

[Florian Fainelli]

ARM's fncpy implementation is actually suitable for a large number of
platforms since the only assumption it makes is just the function's
alignment (8 bytes) which also happens to fulfil other architectures,
including but not limited to ARM64.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/include/asm/fncpy.h | 77 ++----------------------------------
 include/asm-generic/fncpy.h  | 94 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 74 deletions(-)
 create mode 100644 include/asm-generic/fncpy.h

diff --git a/arch/arm/include/asm/fncpy.h b/arch/arm/include/asm/fncpy.h
index de5354746924..32465aef7932 100644
--- a/arch/arm/include/asm/fncpy.h
+++ b/arch/arm/include/asm/fncpy.h
@@ -16,79 +16,8 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
+#ifndef _ASMARM_FNCPY_H
 
-/*
- * 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;							\
-})
+#include <asm-generic/fncpy.h>
 
-#endif /* !__ASM_FNCPY_H */
+#endif /* !__ASMARM_FNCPY_H */
diff --git a/include/asm-generic/fncpy.h b/include/asm-generic/fncpy.h
new file mode 100644
index 000000000000..bf7577725652
--- /dev/null
+++ b/include/asm-generic/fncpy.h
@@ -0,0 +1,94 @@
+/*
+ * include/asm-generic/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 */
-- 
2.9.3

Re: [PATCH 1/3] ARM: Generalize fncpy implementation

[Florian Fainelli]

On 06/15/2017 04:21 PM, Florian Fainelli wrote:
> ARM's fncpy implementation is actually suitable for a large number of
> platforms since the only assumption it makes is just the function's
> alignment (8 bytes) which also happens to fulfil other architectures,
> including but not limited to ARM64.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/include/asm/fncpy.h | 77 ++----------------------------------
>  include/asm-generic/fncpy.h  | 94 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 97 insertions(+), 74 deletions(-)
>  create mode 100644 include/asm-generic/fncpy.h
> 
> diff --git a/arch/arm/include/asm/fncpy.h b/arch/arm/include/asm/fncpy.h
> index de5354746924..32465aef7932 100644
> --- a/arch/arm/include/asm/fncpy.h
> +++ b/arch/arm/include/asm/fncpy.h
> @@ -16,79 +16,8 @@
>   * along with this program; if not, write to the Free Software
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>   */
> +#ifndef _ASMARM_FNCPY_H

and of course I missed a

#define _ASMARM_FNCPY_H

I will wait for feedback on whether this is acceptable before
resubmitting...
-- 
Florian

Re: [PATCH 1/3] ARM: Generalize fncpy implementation

[Tony Lindgren]

* Florian Fainelli <f.fainelli@gmail.com> [170615 16:27]:
> On 06/15/2017 04:21 PM, Florian Fainelli wrote:
> > ARM's fncpy implementation is actually suitable for a large number of
> > platforms since the only assumption it makes is just the function's
> > alignment (8 bytes) which also happens to fulfil other architectures,
> > including but not limited to ARM64.

Yeah I'm all for it. Maybe mention in the next version that this helps
with making SoC specific PM code into just regular Linux device drivers.

Regards,

Tony

Re: [PATCH 1/3] ARM: Generalize fncpy implementation

[Russell King - ARM Linux]

On Thu, Jun 15, 2017 at 04:21:15PM -0700, Florian Fainelli wrote:
> ARM's fncpy implementation is actually suitable for a large number of
> platforms since the only assumption it makes is just the function's
> alignment (8 bytes) which also happens to fulfil other architectures,
> including but not limited to ARM64.

NAK.  This is really not "generic" because the whole point of this is
that it encapsulates architecture specific knowledge - in the case of
ARM, the fact that bit 1 is used to indicate whether the code is to be
run in Thumb mode or ARM mode.

That clearly does not belong in an asm-generic version of this.

I'm not saying "don't provide an asm-generic" version, I'm saying don't
use the ARM version as an asm-generic implementation, because it is
nothing of the sort.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/3] arm64: Provide a fncpy implenentation

[Florian Fainelli]

Utilize the asm-generic/fncpy.h implementation for ARM64 to allow the
use of drivers/misc/sram*.c on these platforms as well.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm64/include/asm/fncpy.h | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 arch/arm64/include/asm/fncpy.h

diff --git a/arch/arm64/include/asm/fncpy.h b/arch/arm64/include/asm/fncpy.h
new file mode 100644
index 000000000000..578f942f55e4
--- /dev/null
+++ b/arch/arm64/include/asm/fncpy.h
@@ -0,0 +1,6 @@
+#ifndef __ASMARM64_FNCPY_H
+#define __ASMARM64_FNCPY_H
+
+#include <asm-generic/fncpy.h>
+
+#endif /* __ASMARM64_FNCPY_H */
-- 
2.9.3

[PATCH 3/3] misc: sram: Allow ARM64 to select SRAM_EXEC

[Florian Fainelli]

Now that ARM64 also has a fncpy() implementation, allow selection
SRAM_EXEC for ARM64 as well.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/misc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 07bbd4cc1852..ac8779278c0c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -464,7 +464,7 @@ config SRAM
 	bool "Generic on-chip SRAM driver"
 	depends on HAS_IOMEM
 	select GENERIC_ALLOCATOR
-	select SRAM_EXEC if ARM
+	select SRAM_EXEC if ARM || ARM64
 	help
 	  This driver allows you to declare a memory region to be managed by
 	  the genalloc API. It is supposed to be used for small on-chip SRAM
-- 
2.9.3