From: Andre Przywara <andre.przywara@arm.com>
Date: Tue, 7 Feb 2023 15:21:04 +0000 (+0000)
Subject: arm: semihosting: replace inline assembly with assembly file
X-Git-Url: http://git.dujemihanovic.xyz/img/static/gitweb.css?a=commitdiff_plain;h=29c579a2493a1c5de162a724e05521099b66bedb;p=u-boot.git

arm: semihosting: replace inline assembly with assembly file

So far we used inline assembly to inject the actual instruction that
triggers the semihosting service. While this sounds elegant, as it's
really only about one instruction, it has some serious downsides:
- We need some barriers in place to force the compiler to issue writes
  to a data structure before issuing the trap instruction.
- We need to convince the compiler to actually fill the structures that
  we use pointers to.
- We need a memory clobber to avoid the compiler caching the data in
  those structures, when semihosting writes data back.
- We need register arguments to make sure the function ID and the
  pointer land in the right registers.

This is all doable, but fragile and somewhat cumbersome. Since we now
have a separate function in an extra file anyway, we can do away with
all the magic and just write that in an actual assembly file.
This is much more readable and robust.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Sean Anderson <sean.anderson@seco.com>
---

diff --git a/arch/arm/lib/semihosting.S b/arch/arm/lib/semihosting.S
new file mode 100644
index 0000000000..393aade94a
--- /dev/null
+++ b/arch/arm/lib/semihosting.S
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * (C) 2022 Arm Ltd.
+ */
+
+#include <config.h>
+#include <asm/macro.h>
+#include <linux/linkage.h>
+
+.pushsection .text.smh_trap, "ax"
+/* long smh_trap(unsigned int sysnum, void *addr); */
+ENTRY(smh_trap)
+
+#if defined(CONFIG_ARM64)
+	hlt	#0xf000
+#elif defined(CONFIG_CPU_V7M)
+	bkpt	#0xab
+#elif defined(CONFIG_SYS_THUMB_BUILD)
+	svc	#0xab
+#else
+	svc	#0x123456
+#endif
+
+#if defined(CONFIG_ARM64)
+	ret
+#else
+	bx	lr
+#endif
+
+ENDPROC(smh_trap)
+.popsection
diff --git a/arch/arm/lib/semihosting.c b/arch/arm/lib/semihosting.c
deleted file mode 100644
index 7b7669bed0..0000000000
--- a/arch/arm/lib/semihosting.c
+++ /dev/null
@@ -1,47 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright (C) 2022 Sean Anderson <sean.anderson@seco.com>
- * Copyright 2014 Broadcom Corporation
- */
-
-#include <common.h>
-
-/*
- * 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)
-	#define SMH_TRAP "bkpt #0xAB"
-#elif defined(CONFIG_SYS_THUMB_BUILD)
-	#define SMH_TRAP "svc #0xab"
-#else
-	#define SMH_TRAP "svc #0x123456"
-#endif
-
-/*
- * Call the handler
- */
-long smh_trap(unsigned int sysnum, void *addr)
-{
-	register long result asm("r0");
-	register void *_addr asm("r1") = addr;
-
-	/*
-	 * 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"(USE_PTR(_addr))
-		      : "memory");
-
-	return result;
-}