Re: [RFC v4 07/11] target/i386: Add support for native library calls

[Richard Henderson <richard.henderson@linaro.org>]

On 8/8/23 07:17, Yeqi Fu wrote:
> This commit introduces support for native library calls on the
> i386 target. When special instructions reserved for native calls
> are encountered, the code now performs address translation and
> generates the corresponding native call.
> 
> Signed-off-by: Yeqi Fu <fufuyqqqqqq@gmail.com>
> ---
>   configs/targets/i386-linux-user.mak   |  1 +
>   configs/targets/x86_64-linux-user.mak |  1 +
>   target/i386/tcg/translate.c           | 27 +++++++++++++++++++++++++++
>   3 files changed, 29 insertions(+)
> 
> diff --git a/configs/targets/i386-linux-user.mak b/configs/targets/i386-linux-user.mak
> index 5b2546a430..2d8bca8f93 100644
> --- a/configs/targets/i386-linux-user.mak
> +++ b/configs/targets/i386-linux-user.mak
> @@ -2,3 +2,4 @@ TARGET_ARCH=i386
>   TARGET_SYSTBL_ABI=i386
>   TARGET_SYSTBL=syscall_32.tbl
>   TARGET_XML_FILES= gdb-xml/i386-32bit.xml
> +CONFIG_NATIVE_CALL=y
> diff --git a/configs/targets/x86_64-linux-user.mak b/configs/targets/x86_64-linux-user.mak
> index 9ceefbb615..a53b017454 100644
> --- a/configs/targets/x86_64-linux-user.mak
> +++ b/configs/targets/x86_64-linux-user.mak
> @@ -3,3 +3,4 @@ TARGET_BASE_ARCH=i386
>   TARGET_SYSTBL_ABI=common,64
>   TARGET_SYSTBL=syscall_64.tbl
>   TARGET_XML_FILES= gdb-xml/i386-64bit.xml
> +CONFIG_NATIVE_CALL=y
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 90c7b32f36..28bf4477fb 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -33,6 +33,7 @@
>   #include "helper-tcg.h"
>   
>   #include "exec/log.h"
> +#include "native/native.h"
>   
>   #define HELPER_H "helper.h"
>   #include "exec/helper-info.c.inc"
> @@ -6810,6 +6811,32 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>       case 0x1d0 ... 0x1fe:
>           disas_insn_new(s, cpu, b);
>           break;
> +    case 0x1ff:
> +        if (native_bypass_enabled()) {
> +            TCGv ret = tcg_temp_new();
> +            TCGv arg1 = tcg_temp_new();
> +            TCGv arg2 = tcg_temp_new();
> +            TCGv arg3 = tcg_temp_new();
> +            const char *fun_name = lookup_symbol((s->base.pc_next) & 0xfff);

I'm not keen on this lookup_symbol interface.
I would much rather there be some data encoded in the native.so.


> +            uintptr_t ra = GETPC();
> +            uint32_t a1 = cpu_ldl_data_ra(env, env->regs[R_ESP] + 4, ra);
> +            uint32_t a2 = cpu_ldl_data_ra(env, env->regs[R_ESP] + 8, ra);
> +            uint32_t a3 = cpu_ldl_data_ra(env, env->regs[R_ESP] + 12, ra);
> +            tcg_gen_movi_tl(arg1, a1);
> +            tcg_gen_movi_tl(arg2, a2);
> +            tcg_gen_movi_tl(arg3, a3);

This is wrong.  You are performing the stack load at translation time, but it must be done 
at execution time.  You need

	tcg_gen_addi_tl(arg1, cpu_regs[R_ESP], 4);  /* arg1 = esp + 4 */
	gen_op_ld_v(s, MO_UL, arg1, arg1);          /* arg1 = *arg1 */

etc.


r~