libpayload: Use Kconfig for architecture memcpy, not weak symbols

Our mechanism to override the default (pure C) memory function
implementations (memset, memcpy, memmove) with architecture-specific
optimized assembly versions doesn't actually work: it turns out that
weak functions don't work as you'd naively expect when you pack them
together with a strong definition from a different object into a static
library. When a linker tries to resolve a symbol from a static library,
it just picks the first one it finds, even if it is weak. It doesn't
evaluate all objects in the library to see if there are other strong
definitions.

To fix this, this patch gets rid of the weak symbols and uses Kconfigs
instead. It adds an optimized memmove() implementation for x86 because
that makes things easier (then all architectures either override all
three functions or none of them). Also remove memcmp() from the
functions that can be overridden for now because nobody ever needed that
anyway.

Change-Id: Iedf9898247f1999e56fde3233fad8b7cb36b1269
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87766
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

payloads/libpayload/Kconfig

@@ -505,6 +505,13 @@ config IO_ADDRESS_SPACE
 	  This option is turned on if the target system has a separate
 	  IO address space. This is typically only the case on x86.
 
+config ARCH_HAS_MEM_FUNCTIONS
+	default n
+	bool
+	help
+	  Architectures must select this when they want to override memset,
+	  memcpy and memmove with optimized assembly implementations.
+
 source "arch/arm/Kconfig"
 source "arch/arm64/Kconfig"
 source "arch/x86/Kconfig"

payloads/libpayload/arch/arm/Kconfig

@@ -30,3 +30,4 @@
 
 config ARCH_ARM
 	select LITTLE_ENDIAN
+	select ARCH_HAS_MEM_FUNCTIONS if GPL

payloads/libpayload/arch/arm64/Kconfig

@@ -30,6 +30,7 @@
 
 config ARCH_ARM64
 	select LITTLE_ENDIAN
+	select ARCH_HAS_MEM_FUNCTIONS
 
 if ARCH_ARM64
 

payloads/libpayload/arch/x86/Kconfig

@@ -31,6 +31,7 @@
 config ARCH_X86
 	select LITTLE_ENDIAN
 	select IO_ADDRESS_SPACE
+	select ARCH_HAS_MEM_FUNCTIONS if GPL
 
 if ARCH_X86
 

payloads/libpayload/arch/x86/string.c

@@ -103,3 +103,45 @@ void *memcpy(void *dest, const void *src, size_t n)
 
 	return dest;
 }
+
+void *memmove(void *dest, const void *src, size_t n)
+{
+	unsigned long d0, d1, d2;
+
+	if (dest < src)
+		return memcpy(dest, src, n);
+
+#if CONFIG(LP_ARCH_X86_64)
+	asm volatile(
+		"std\n\t"
+		"dec %%rdi\n\t"
+		"dec %%rsi\n\t"
+		"rep ; movsb\n\t"
+		"mov %4, %%rcx\n\t"
+		"sub $7, %%rdi\n\t"
+		"sub $7, %%rsi\n\t"
+		"rep ; movsq\n\t"
+		"cld\n\t"
+		: "=&c" (d0), "=&D" (d1), "=&S" (d2)
+		: "0" (n & 7), "g" (n >> 3), "1" (dest + n), "2" (src + n)
+		: "memory"
+	);
+#else
+	asm volatile(
+		"std\n\t"
+		"dec %%edi\n\t"
+		"dec %%esi\n\t"
+		"rep ; movsb\n\t"
+		"mov %4, %%ecx\n\t"
+		"sub $3, %%edi\n\t"
+		"sub $3, %%esi\n\t"
+		"rep ; movsl\n\t"
+		"cld\n\t"
+		: "=&c" (d0), "=&D" (d1), "=&S" (d2)
+		: "0" (n & 3), "g" (n >> 2), "1" (dest + n), "2" (src + n)
+		: "memory"
+	);
+#endif
+
+	return dest;
+}

payloads/libpayload/include/string.h

@@ -43,6 +43,15 @@ void *memchr(const void *s, int c, size_t n);
 int memcmp(const void *s1, const void *s2, size_t len);
 /** @} */
 
+/**
+ * @defgroup default memory functions remain available under separate names, in
+ * case architecture implementations want to fall back to them in certain cases.
+ */
+void *default_memset(void *s, int c, size_t n);
+void *default_memcpy(void *dst, const void *src, size_t n);
+void *default_memmove(void *dst, const void *src, size_t n);
+/** @} */
+
 /**
  * @defgroup string String functions
  * @{

payloads/libpayload/libc/memory.c

@@ -32,7 +32,7 @@
 
 #include <libpayload.h>
 
-static void *default_memset(void *const s, const int c, size_t n)
+void *default_memset(void *const s, const int c, size_t n)
 {
 	size_t i;
 	u8 *dst = s;
@@ -57,10 +57,7 @@ static void *default_memset(void *const s, const int c, size_t n)
 	return s;
 }
 
-void *memset(void *s, int c, size_t n)
-	__attribute__((weak, alias("default_memset")));
-
-static void *default_memcpy(void *dst, const void *src, size_t n)
+void *default_memcpy(void *dst, const void *src, size_t n)
 {
 	size_t i;
 	void *ret = dst;
@@ -81,10 +78,7 @@ static void *default_memcpy(void *dst, const void *src, size_t n)
 	return ret;
 }
 
-void *memcpy(void *dst, const void *src, size_t n)
-	__attribute__((weak, alias("default_memcpy")));
-
-static void *default_memmove(void *dst, const void *src, size_t n)
+void *default_memmove(void *dst, const void *src, size_t n)
 {
 	size_t offs;
 	ssize_t i;
@@ -110,8 +104,14 @@ static void *default_memmove(void *dst, const void *src, size_t n)
 	return dst;
 }
 
+#if !CONFIG(LP_ARCH_HAS_MEM_FUNCTIONS)
+void *memcpy(void *dst, const void *src, size_t n)
+	__attribute__((alias("default_memcpy")));
+void *memset(void *s, int c, size_t n)
+	__attribute__((alias("default_memset")));
 void *memmove(void *dst, const void *src, size_t n)
-	__attribute__((weak, alias("default_memmove")));
+	__attribute__((alias("default_memmove")));
+#endif
 
 /**
  * Compare two memory areas.
@@ -124,7 +124,7 @@ void *memmove(void *dst, const void *src, size_t n)
  * 	   greater than s2 respectively.
  */
 
-static int default_memcmp(const void *s1, const void *s2, size_t n)
+int memcmp(const void *s1, const void *s2, size_t n)
 {
 	size_t i = 0;
 	const unsigned long *w1 = s1, *w2 = s2;
@@ -142,9 +142,6 @@ static int default_memcmp(const void *s1, const void *s2, size_t n)
 	return 0;
 }
 
-int memcmp(const void *s1, const void *s2, size_t n)
-	__attribute__((weak, alias("default_memcmp")));
-
 void *memchr(const void *s, int c, size_t n)
 {
 	unsigned char *p = (unsigned char *)s;