From: Andre Przywara <andre.przywara@arm.com>
Date: Wed, 5 Oct 2022 16:38:48 +0000 (+0100)
Subject: arm: smh: Make semihosting trap calls more robust
X-Git-Url: http://git.dujemihanovic.xyz/?a=commitdiff_plain;h=d660a82934fdb8ab23a789d1e53ac34825e9f7c7;p=u-boot.git

arm: smh: Make semihosting trap calls more robust

Commit f4b540e25c5c("arm: smh: Fix uninitialized parameters with newer
GCCs") added a memory clobber to the semihosting inline assembly trap
calls, to avoid too eager GCC optimisation: when passing a pointer, newer
compilers couldn't be bothered to actually fill in the structure that it
pointed to, as this data would seemingly never be used (at least from the
compiler's point of view).
But instead of the memory clobber we need to tell the compiler that we are
passing an *array* instead of some generic pointer, this forces the
compiler to actually populate the data structure.
This involves some rather hideous cast, which is best hidden in a macro.

But regardless of that, we actually need the memory clobber, but for two
different reasons: explain them in comments.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---

diff --git a/arch/arm/lib/semihosting.c b/arch/arm/lib/semihosting.c
index acc6b1be4f..3dee7d51b3 100644
--- a/arch/arm/lib/semihosting.c
+++ b/arch/arm/lib/semihosting.c
@@ -20,6 +20,12 @@
 #define SYSFLEN		0x0C
 #define SYSERRNO	0x13
 
+/*
+ * Macro to force the compiler to *populate* memory (for an array or struct)
+ * before passing the pointer to an inline assembly call.
+ */
+#define USE_PTR(ptr) *(const char (*)[]) (ptr)
+
 #if defined(CONFIG_ARM64)
 	#define SMH_TRAP "hlt #0xf000"
 #elif defined(CONFIG_CPU_V7M)
@@ -37,9 +43,17 @@ static noinline long smh_trap(unsigned int sysnum, void *addr)
 {
 	register long result asm("r0");
 
+	/*
+	 * We need a memory clobber (aka compiler barrier) for two reasons:
+	 * - The compiler needs to populate any data structures pointed to
+	 *   by "addr" *before* the trap instruction is called.
+	 * - At least the SYSREAD function puts the result into memory pointed
+	 *   to by "addr", so the compiler must not use a cached version of
+	 *   the previous content, after the call has finished.
+	 */
 	asm volatile (SMH_TRAP
 		      : "=r" (result)
-		      : "0"(sysnum), "r"(addr)
+		      : "0"(sysnum), "r"(USE_PTR(addr))
 		      : "memory");
 
 	return result;