[2/2] riscv: 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 a few instructions, 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 assembler.
This is much more readable and robust.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/riscv/lib/semihosting.S | 22 ++++++++++++++++++++++
 arch/riscv/lib/semihosting.c | 24 ------------------------
 2 files changed, 22 insertions(+), 24 deletions(-)
 create mode 100644 arch/riscv/lib/semihosting.S
 delete mode 100644 arch/riscv/lib/semihosting.c

On 2/7/23 10:21, Andre Przywara wrote:
> 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 a few instructions, 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 assembler.
> This is much more readable and robust.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/riscv/lib/semihosting.S | 22 ++++++++++++++++++++++
>  arch/riscv/lib/semihosting.c | 24 ------------------------
>  2 files changed, 22 insertions(+), 24 deletions(-)
>  create mode 100644 arch/riscv/lib/semihosting.S
>  delete mode 100644 arch/riscv/lib/semihosting.c
> 
> diff --git a/arch/riscv/lib/semihosting.S b/arch/riscv/lib/semihosting.S
> new file mode 100644
> index 00000000000..5fff485b875
> --- /dev/null
> +++ b/arch/riscv/lib/semihosting.S
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2022 Ventana Micro Systems Inc.
> + */
> +
> +#include <asm/asm.h>
> +#include <linux/linkage.h>
> +
> +.pushsection .text.smh_trap, "ax"
> +ENTRY(smh_trap)
> +	.align	2
> +	.option	push
> +	.option	norvc		/* semihosting sequence must be 32-bit wide */
> +
> +	slli zero, zero, 0x1f	/* Entry NOP to identify semihosting */
> +	ebreak
> +	srai zero, zero, 7	/* NOP encoding of semihosting call number */
> +	.option pop
> +
> +	ret
> +ENDPROC(smh_trap)
> +.popsection
> diff --git a/arch/riscv/lib/semihosting.c b/arch/riscv/lib/semihosting.c
> deleted file mode 100644
> index d6593b02a6f..00000000000
> --- a/arch/riscv/lib/semihosting.c
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Copyright (C) 2022 Ventana Micro Systems Inc.
> - */
> -
> -#include <common.h>
> -
> -long smh_trap(int sysnum, void *addr)
> -{
> -	register int ret asm ("a0") = sysnum;
> -	register void *param0 asm ("a1") = addr;
> -
> -	asm volatile (".align 4\n"
> -		".option push\n"
> -		".option norvc\n"
> -
> -		"slli zero, zero, 0x1f\n"
> -		"ebreak\n"
> -		"srai zero, zero, 7\n"
> -		".option pop\n"
> -		: "+r" (ret) : "r" (param0) : "memory");
> -
> -	return ret;
> -}

Reviewed-by: Sean Anderson <sean.anderson@seco.com>

On Tue, Feb 07, 2023 at 03:21:05PM +0000, Andre Przywara wrote:

> 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 a few instructions, 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 assembler.
> This is much more readable and robust.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reviewed-by: Sean Anderson <sean.anderson@seco.com>

After correcting the style on the SPDX header of the new .S file,
applied to u-boot/next, thanks!