[PATCH 2/2] riscv: semihosting: replace inline assembly with assembly file

[Sean Anderson]

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 at 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 at seco.com>