diff mbox series

[v2,1/7] linux-user/nios2: Properly emulate EXCP_TRAP

Message ID 20211221025012.1057923-2-richard.henderson@linaro.org
State New
Headers show
Series linux-user: nios2 fixes | expand

Commit Message

Richard Henderson Dec. 21, 2021, 2:50 a.m. UTC 
The real kernel has to load the instruction and extract
the imm5 field; for qemu, modify the translator to do this.

The use of R_AT for this in cpu_loop was a bug.  Handle
the other trap numbers as per the kernel's trap_table.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/cpu.h          |  2 +-
 linux-user/nios2/cpu_loop.c | 40 ++++++++++++++++++++-----------------
 target/nios2/translate.c    | 17 +++++++++++++++-
 3 files changed, 39 insertions(+), 20 deletions(-)

Comments

Laurent Vivier Dec. 22, 2021, 6:52 p.m. UTC | #1 
Le 21/12/2021 à 03:50, Richard Henderson a écrit :
> The real kernel has to load the instruction and extract
> the imm5 field; for qemu, modify the translator to do this.
> 
> The use of R_AT for this in cpu_loop was a bug.  Handle
> the other trap numbers as per the kernel's trap_table.
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/nios2/cpu.h          |  2 +-
>   linux-user/nios2/cpu_loop.c | 40 ++++++++++++++++++++-----------------
>   target/nios2/translate.c    | 17 +++++++++++++++-
>   3 files changed, 39 insertions(+), 20 deletions(-)
> 
> diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
> index 1a69ed7a49..d2ba0c5bbd 100644
> --- a/target/nios2/cpu.h
> +++ b/target/nios2/cpu.h
> @@ -160,9 +160,9 @@ struct CPUNios2State {
>   
>   #if !defined(CONFIG_USER_ONLY)
>       Nios2MMU mmu;
> -
>       uint32_t irq_pending;
>   #endif
> +    int error_code;
>   };
>   
>   /**
> diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
> index 34290fb3b5..5c3d01d22d 100644
> --- a/linux-user/nios2/cpu_loop.c
> +++ b/linux-user/nios2/cpu_loop.c
> @@ -26,7 +26,6 @@
>   void cpu_loop(CPUNios2State *env)
>   {
>       CPUState *cs = env_cpu(env);
> -    Nios2CPU *cpu = NIOS2_CPU(cs);
>       target_siginfo_t info;
>       int trapnr, ret;
>   
> @@ -39,9 +38,10 @@ void cpu_loop(CPUNios2State *env)
>           case EXCP_INTERRUPT:
>               /* just indicate that signals should be handled asap */
>               break;
> +
>           case EXCP_TRAP:
> -            if (env->regs[R_AT] == 0) {
> -                abi_long ret;
> +            switch (env->error_code) {
> +            case 0:
>                   qemu_log_mask(CPU_LOG_INT, "\nSyscall\n");
>   
>                   ret = do_syscall(env, env->regs[2],
> @@ -55,26 +55,30 @@ void cpu_loop(CPUNios2State *env)
>   
>                   env->regs[2] = abs(ret);
>                   /* Return value is 0..4096 */
> -                env->regs[7] = (ret > 0xfffffffffffff000ULL);
> -                env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
> -                env->regs[CR_STATUS] &= ~0x3;
> -                env->regs[R_EA] = env->regs[R_PC] + 4;

I think this change above deserves more explanation.

I guess CR_ESTATUS is never used in linux-user mode and R_EA is set by nios2_cpu_do_interrupt().

Moreover It doesn't seem supervisor-only instructiond like eret (that uses CR_ESTATUS) generates an 
exception in linux-user mode.

Anyway, it looks good:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

> +                env->regs[7] = ret > 0xfffff000u;
>                   env->regs[R_PC] += 4;
>                   break;
> -            } else {
> -                qemu_log_mask(CPU_LOG_INT, "\nTrap\n");
>   
> -                env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
> -                env->regs[CR_STATUS] &= ~0x3;
> -                env->regs[R_EA] = env->regs[R_PC] + 4;
> -                env->regs[R_PC] = cpu->exception_addr;
> -
> -                info.si_signo = TARGET_SIGTRAP;
> -                info.si_errno = 0;
> -                info.si_code = TARGET_TRAP_BRKPT;
> -                queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +            case 1:
> +                qemu_log_mask(CPU_LOG_INT, "\nTrap 1\n");
> +                force_sig_fault(TARGET_SIGUSR1, 0, env->regs[R_PC]);
> +                break;
> +            case 2:
> +                qemu_log_mask(CPU_LOG_INT, "\nTrap 2\n");
> +                force_sig_fault(TARGET_SIGUSR2, 0, env->regs[R_PC]);
> +                break;
> +            case 31:
> +                qemu_log_mask(CPU_LOG_INT, "\nTrap 31\n");
> +                force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->regs[R_PC]);
> +                break;
> +            default:
> +                qemu_log_mask(CPU_LOG_INT, "\nTrap %d\n", env->error_code);
> +                force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLTRP,
> +                                env->regs[R_PC]);
>                   break;
>               }
> +            break;
> +
>           case EXCP_DEBUG:
>               info.si_signo = TARGET_SIGTRAP;
>               info.si_errno = 0;
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index 08d7ac5398..a759877519 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -636,6 +636,21 @@ static void divu(DisasContext *dc, uint32_t code, uint32_t flags)
>       tcg_temp_free(t0);
>   }
>   
> +static void trap(DisasContext *dc, uint32_t code, uint32_t flags)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    /*
> +     * The imm5 field is not stored anywhere on real hw; the kernel
> +     * has to load the insn and extract the field.  But we can make
> +     * things easier for cpu_loop if we pop this into env->error_code.
> +     */
> +    R_TYPE(instr, code);
> +    tcg_gen_st_i32(tcg_constant_i32(instr.imm5), cpu_env,
> +                   offsetof(CPUNios2State, error_code));
> +#endif
> +    t_gen_helper_raise_exception(dc, EXCP_TRAP);
> +}
> +
>   static const Nios2Instruction r_type_instructions[] = {
>       INSTRUCTION_ILLEGAL(),
>       INSTRUCTION(eret),                                /* eret */
> @@ -682,7 +697,7 @@ static const Nios2Instruction r_type_instructions[] = {
>       INSTRUCTION_ILLEGAL(),
>       INSTRUCTION_ILLEGAL(),
>       INSTRUCTION_ILLEGAL(),
> -    INSTRUCTION_FLG(gen_excp, EXCP_TRAP),             /* trap */
> +    INSTRUCTION(trap),                                /* trap */
>       INSTRUCTION(wrctl),                               /* wrctl */
>       INSTRUCTION_ILLEGAL(),
>       INSTRUCTION_FLG(gen_cmpxx, TCG_COND_LTU),         /* cmpltu */

Thanks,
Laurent
Laurent Vivier Jan. 5, 2022, 10:23 a.m. UTC | #2 
Le 21/12/2021 à 03:50, Richard Henderson a écrit :
> The real kernel has to load the instruction and extract
> the imm5 field; for qemu, modify the translator to do this.
> 
> The use of R_AT for this in cpu_loop was a bug.  Handle
> the other trap numbers as per the kernel's trap_table.
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/nios2/cpu.h          |  2 +-
>   linux-user/nios2/cpu_loop.c | 40 ++++++++++++++++++++-----------------
>   target/nios2/translate.c    | 17 +++++++++++++++-
>   3 files changed, 39 insertions(+), 20 deletions(-)
> 
> diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
> index 1a69ed7a49..d2ba0c5bbd 100644
> --- a/target/nios2/cpu.h
> +++ b/target/nios2/cpu.h
> @@ -160,9 +160,9 @@ struct CPUNios2State {
>   
>   #if !defined(CONFIG_USER_ONLY)
>       Nios2MMU mmu;
> -
>       uint32_t irq_pending;
>   #endif
> +    int error_code;
>   };
>   
>   /**
> diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
> index 34290fb3b5..5c3d01d22d 100644
> --- a/linux-user/nios2/cpu_loop.c
> +++ b/linux-user/nios2/cpu_loop.c
> @@ -26,7 +26,6 @@
>   void cpu_loop(CPUNios2State *env)
>   {
>       CPUState *cs = env_cpu(env);
> -    Nios2CPU *cpu = NIOS2_CPU(cs);
>       target_siginfo_t info;
>       int trapnr, ret;
>   
> @@ -39,9 +38,10 @@ void cpu_loop(CPUNios2State *env)
>           case EXCP_INTERRUPT:
>               /* just indicate that signals should be handled asap */
>               break;
> +
>           case EXCP_TRAP:
> -            if (env->regs[R_AT] == 0) {
> -                abi_long ret;
> +            switch (env->error_code) {
> +            case 0:
>                   qemu_log_mask(CPU_LOG_INT, "\nSyscall\n");
>   
>                   ret = do_syscall(env, env->regs[2],
> @@ -55,26 +55,30 @@ void cpu_loop(CPUNios2State *env)
>   
>                   env->regs[2] = abs(ret);
>                   /* Return value is 0..4096 */
> -                env->regs[7] = (ret > 0xfffffffffffff000ULL);
> -                env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
> -                env->regs[CR_STATUS] &= ~0x3;
> -                env->regs[R_EA] = env->regs[R_PC] + 4;
> +                env->regs[7] = ret > 0xfffff000u;
>                   env->regs[R_PC] += 4;
>                   break;
> -            } else {
> -                qemu_log_mask(CPU_LOG_INT, "\nTrap\n");
>   
> -                env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
> -                env->regs[CR_STATUS] &= ~0x3;
> -                env->regs[R_EA] = env->regs[R_PC] + 4;
> -                env->regs[R_PC] = cpu->exception_addr;
> -
> -                info.si_signo = TARGET_SIGTRAP;
> -                info.si_errno = 0;
> -                info.si_code = TARGET_TRAP_BRKPT;
> -                queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +            case 1:
> +                qemu_log_mask(CPU_LOG_INT, "\nTrap 1\n");
> +                force_sig_fault(TARGET_SIGUSR1, 0, env->regs[R_PC]);
> +                break;
> +            case 2:
> +                qemu_log_mask(CPU_LOG_INT, "\nTrap 2\n");
> +                force_sig_fault(TARGET_SIGUSR2, 0, env->regs[R_PC]);
> +                break;
> +            case 31:
> +                qemu_log_mask(CPU_LOG_INT, "\nTrap 31\n");
> +                force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->regs[R_PC]);
> +                break;
> +            default:
> +                qemu_log_mask(CPU_LOG_INT, "\nTrap %d\n", env->error_code);
> +                force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLTRP,
> +                                env->regs[R_PC]);
>                   break;
>               }
> +            break;
> +
>           case EXCP_DEBUG:
>               info.si_signo = TARGET_SIGTRAP;
>               info.si_errno = 0;
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index 08d7ac5398..a759877519 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -636,6 +636,21 @@ static void divu(DisasContext *dc, uint32_t code, uint32_t flags)
>       tcg_temp_free(t0);
>   }
>   
> +static void trap(DisasContext *dc, uint32_t code, uint32_t flags)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    /*
> +     * The imm5 field is not stored anywhere on real hw; the kernel
> +     * has to load the insn and extract the field.  But we can make
> +     * things easier for cpu_loop if we pop this into env->error_code.
> +     */
> +    R_TYPE(instr, code);
> +    tcg_gen_st_i32(tcg_constant_i32(instr.imm5), cpu_env,
> +                   offsetof(CPUNios2State, error_code));
> +#endif
> +    t_gen_helper_raise_exception(dc, EXCP_TRAP);
> +}
> +
>   static const Nios2Instruction r_type_instructions[] = {
>       INSTRUCTION_ILLEGAL(),
>       INSTRUCTION(eret),                                /* eret */
> @@ -682,7 +697,7 @@ static const Nios2Instruction r_type_instructions[] = {
>       INSTRUCTION_ILLEGAL(),
>       INSTRUCTION_ILLEGAL(),
>       INSTRUCTION_ILLEGAL(),
> -    INSTRUCTION_FLG(gen_excp, EXCP_TRAP),             /* trap */
> +    INSTRUCTION(trap),                                /* trap */
>       INSTRUCTION(wrctl),                               /* wrctl */
>       INSTRUCTION_ILLEGAL(),
>       INSTRUCTION_FLG(gen_cmpxx, TCG_COND_LTU),         /* cmpltu */

Applied to my linux-user-for-7.0 branch.

Thanks,
Laurent
diff mbox series

Patch

diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index 1a69ed7a49..d2ba0c5bbd 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -160,9 +160,9 @@  struct CPUNios2State {
 
 #if !defined(CONFIG_USER_ONLY)
     Nios2MMU mmu;
-
     uint32_t irq_pending;
 #endif
+    int error_code;
 };
 
 /**
diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
index 34290fb3b5..5c3d01d22d 100644
--- a/linux-user/nios2/cpu_loop.c
+++ b/linux-user/nios2/cpu_loop.c
@@ -26,7 +26,6 @@ 
 void cpu_loop(CPUNios2State *env)
 {
     CPUState *cs = env_cpu(env);
-    Nios2CPU *cpu = NIOS2_CPU(cs);
     target_siginfo_t info;
     int trapnr, ret;
 
@@ -39,9 +38,10 @@  void cpu_loop(CPUNios2State *env)
         case EXCP_INTERRUPT:
             /* just indicate that signals should be handled asap */
             break;
+
         case EXCP_TRAP:
-            if (env->regs[R_AT] == 0) {
-                abi_long ret;
+            switch (env->error_code) {
+            case 0:
                 qemu_log_mask(CPU_LOG_INT, "\nSyscall\n");
 
                 ret = do_syscall(env, env->regs[2],
@@ -55,26 +55,30 @@  void cpu_loop(CPUNios2State *env)
 
                 env->regs[2] = abs(ret);
                 /* Return value is 0..4096 */
-                env->regs[7] = (ret > 0xfffffffffffff000ULL);
-                env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
-                env->regs[CR_STATUS] &= ~0x3;
-                env->regs[R_EA] = env->regs[R_PC] + 4;
+                env->regs[7] = ret > 0xfffff000u;
                 env->regs[R_PC] += 4;
                 break;
-            } else {
-                qemu_log_mask(CPU_LOG_INT, "\nTrap\n");
 
-                env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
-                env->regs[CR_STATUS] &= ~0x3;
-                env->regs[R_EA] = env->regs[R_PC] + 4;
-                env->regs[R_PC] = cpu->exception_addr;
-
-                info.si_signo = TARGET_SIGTRAP;
-                info.si_errno = 0;
-                info.si_code = TARGET_TRAP_BRKPT;
-                queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+            case 1:
+                qemu_log_mask(CPU_LOG_INT, "\nTrap 1\n");
+                force_sig_fault(TARGET_SIGUSR1, 0, env->regs[R_PC]);
+                break;
+            case 2:
+                qemu_log_mask(CPU_LOG_INT, "\nTrap 2\n");
+                force_sig_fault(TARGET_SIGUSR2, 0, env->regs[R_PC]);
+                break;
+            case 31:
+                qemu_log_mask(CPU_LOG_INT, "\nTrap 31\n");
+                force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->regs[R_PC]);
+                break;
+            default:
+                qemu_log_mask(CPU_LOG_INT, "\nTrap %d\n", env->error_code);
+                force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLTRP,
+                                env->regs[R_PC]);
                 break;
             }
+            break;
+
         case EXCP_DEBUG:
             info.si_signo = TARGET_SIGTRAP;
             info.si_errno = 0;
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 08d7ac5398..a759877519 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -636,6 +636,21 @@  static void divu(DisasContext *dc, uint32_t code, uint32_t flags)
     tcg_temp_free(t0);
 }
 
+static void trap(DisasContext *dc, uint32_t code, uint32_t flags)
+{
+#ifdef CONFIG_USER_ONLY
+    /*
+     * The imm5 field is not stored anywhere on real hw; the kernel
+     * has to load the insn and extract the field.  But we can make
+     * things easier for cpu_loop if we pop this into env->error_code.
+     */
+    R_TYPE(instr, code);
+    tcg_gen_st_i32(tcg_constant_i32(instr.imm5), cpu_env,
+                   offsetof(CPUNios2State, error_code));
+#endif
+    t_gen_helper_raise_exception(dc, EXCP_TRAP);
+}
+
 static const Nios2Instruction r_type_instructions[] = {
     INSTRUCTION_ILLEGAL(),
     INSTRUCTION(eret),                                /* eret */
@@ -682,7 +697,7 @@  static const Nios2Instruction r_type_instructions[] = {
     INSTRUCTION_ILLEGAL(),
     INSTRUCTION_ILLEGAL(),
     INSTRUCTION_ILLEGAL(),
-    INSTRUCTION_FLG(gen_excp, EXCP_TRAP),             /* trap */
+    INSTRUCTION(trap),                                /* trap */
     INSTRUCTION(wrctl),                               /* wrctl */
     INSTRUCTION_ILLEGAL(),
     INSTRUCTION_FLG(gen_cmpxx, TCG_COND_LTU),         /* cmpltu */