Re: [PATCH v4 17/21] LoongArch Linux User Emulation

From: Richard Henderson <richard.henderson@linaro.org>
To: Song Gao <gaosong@loongson.cn>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, yangxiaojuan@loongson.cn,
	david@redhat.com, bin.meng@windriver.com,
	mark.cave-ayland@ilande.co.uk, aleksandar.rikalo@syrmia.com,
	jcmvbkbc@gmail.com, tsimpson@quicinc.com,
	alistair.francis@wdc.com, edgar.iglesias@gmail.com,
	chenhuacai@gmail.com, philmd@redhat.com, atar4qemu@gmail.com,
	thuth@redhat.com, ehabkost@redhat.com, groug@kaod.org,
	maobibo@loongson.cn, mrolnik@gmail.com, shorne@gmail.com,
	alex.bennee@linaro.org, david@gibson.dropbear.id.au,
	kbastian@mail.uni-paderborn.de, crwulff@gmail.com,
	laurent@vivier.eu, palmer@dabbelt.com, pbonzini@redhat.com,
	aurelien@aurel32.net
Subject: Re: [PATCH v4 17/21] LoongArch Linux User Emulation
Date: Sun, 5 Sep 2021 12:04:38 +0200	[thread overview]
Message-ID: <4e47e1a7-d946-f8ec-a9a1-aadc6eeb7941@linaro.org> (raw)
In-Reply-To: <1630586467-22463-18-git-send-email-gaosong@loongson.cn>

On 9/2/21 2:41 PM, Song Gao wrote:
> +#include "qemu/osdep.h"
> +#include "qemu.h"
> +#include "qemu-common.h"
> +#include "cpu_loop-common.h"
> +#include "elf.h"
> +
> +void cpu_loop(CPULoongArchState *env)
> +{
> +    CPUState *cs = env_cpu(env);
> +    target_siginfo_t info;
> +    int trapnr;
> +    abi_long ret;
> +
> +    for (;;) {
> +        cpu_exec_start(cs);
> +        trapnr = cpu_exec(cs);
> +        cpu_exec_end(cs);
> +        process_queued_cpu_work(cs);
> +
> +        switch (trapnr) {
> +        case EXCP_INTERRUPT:
> +            /* just indicate that signals should be handled asap */
> +            break;
> +        case EXCP_SYSCALL:
> +            env->pc += 4;
> +            ret = do_syscall(env, env->gpr[11],
> +                             env->gpr[4], env->gpr[5],
> +                             env->gpr[6], env->gpr[7],
> +                             env->gpr[8], env->gpr[9],
> +                             -1, -1);
> +            if (ret == -TARGET_ERESTARTSYS) {
> +                env->pc -= 4;
> +                break;
> +            }
> +            if (ret == -TARGET_QEMU_ESIGRETURN) {
> +                /*
> +                 * Returning from a successful sigreturn syscall.
> +                 * Avoid clobbering register state.
> +                 */
> +                break;
> +            }
> +            env->gpr[4] = ret;
> +            break;
> +        case EXCP_ADE:
> +            info.si_signo = TARGET_SIGSEGV;
> +            info.si_errno = 0;
> +            info.si_code = TARGET_SEGV_MAPERR;
> +            info._sifields._sigfault._addr = env->badaddr;
> +            queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +            break;
> +        case EXCP_INE:
> +            info.si_signo = TARGET_SIGILL;
> +            info.si_errno = 0;
> +            info.si_code = 0;
> +            queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);

Missing _addr = pc.

> +            break;
> +        case EXCP_FPE:
> +            info.si_signo = TARGET_SIGFPE;
> +            info.si_errno = 0;
> +            info.si_code = TARGET_FPE_FLTUNK;
> +            if (GET_FP_CAUSE(env->fcsr0) & FP_INVALID) {
> +                info.si_code = TARGET_FPE_FLTINV;
> +            } else if (GET_FP_CAUSE(env->fcsr0) & FP_DIV0) {
> +                info.si_code = TARGET_FPE_FLTDIV;
> +            } else if (GET_FP_CAUSE(env->fcsr0) & FP_OVERFLOW) {
> +                info.si_code = TARGET_FPE_FLTOVF;
> +            } else if (GET_FP_CAUSE(env->fcsr0) & FP_UNDERFLOW) {
> +                info.si_code = TARGET_FPE_FLTUND;
> +            } else if (GET_FP_CAUSE(env->fcsr0) & FP_INEXACT) {
> +                info.si_code = TARGET_FPE_FLTRES;
> +            }
> +            queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);

Likewise.

> +        case EXCP_BREAK:
> +            info.si_signo = TARGET_SIGTRAP;
> +            info.si_code = TARGET_TRAP_BRKPT;
> +            queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);

Likewise.

Note that there are a set of patches on list that introduce a force_sig_fault(signo, code, 
addr) function, so that none of these parameters get missed.

Missing an entry for EXCP_DEBUG, which should share code with EXCP_BREAK.

> +struct sigframe {
> +    uint32_t sf_ass[4];             /* argument save space for o32 */

Surely there is no "o32" for loongarch?

> +    uint32_t sf_code[2];            /* signal trampoline */

Note that there are patches on-list for moving the signal trampoline off of the stack.

> diff --git a/linux-user/loongarch64/termbits.h b/linux-user/loongarch64/termbits.h
> new file mode 100644
> index 0000000..33e74ed
> --- /dev/null
> +++ b/linux-user/loongarch64/termbits.h
> @@ -0,0 +1,229 @@
> +#ifndef LINUX_USER_LOONGARCH_TERMBITS_H
> +#define LINUX_USER_LOONGARCH_TERMBITS_H
> +
> +#define TARGET_NCCS 19

Surely you should be using generic/termbits.h?

We will prefer not to merge a linux-user port that is not upstream, because the ABI may 
change in between.  Can you provide a pointer to your kernel port in the meantime?

r~