[PATCH 0/2] linux-user/nios2: trap and kuser fixes

[PATCH 0/2] linux-user/nios2: trap and kuser fixes

[Richard Henderson]

Based-on: 20210813131809.28655-1-peter.maydell@linaro.org
("linux-user: Clean up siginfo_t handling for arm, aarch64")
... for force_sig_fault().

Emulation of EXCP_TRAP was really wrong, in that we were using
the contents of R1 when we really wanted the trap immediate.

Emulation of the kuser page was wrong, in that the first word
contains the version, and must be readable.  The easiest way
to fix this is to map real data, but then we have to come up
with some replacement for __kuser_cmpxchg, and the easiest way
to do that is to steal an unused trap number.

Both of these stand in the way of cleaning up SIGSEGV, which
is how the kuser page is currently implemented.

I built a nios2-elf binutils and wrote a couple of asm smoke
tests for this.  So, golden!


r~


Richard Henderson (2):
  linux-user/nios2: Properly emulate EXCP_TRAP
  linux-user/nios2: Map a real kuser page

 target/nios2/cpu.h          |  5 ++-
 linux-user/elfload.c        | 35 +++++++++++++++
 linux-user/nios2/cpu_loop.c | 85 ++++++++++++++++++++-----------------
 target/nios2/translate.c    | 26 +++++++-----
 4 files changed, 99 insertions(+), 52 deletions(-)

-- 
2.25.1

[PATCH 1/2] linux-user/nios2: Properly emulate EXCP_TRAP

[Richard Henderson]

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.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/cpu.h          |  5 +++--
 linux-user/nios2/cpu_loop.c | 35 ++++++++++++++++++-----------------
 target/nios2/translate.c    | 17 ++++++++++++++++-
 3 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index 2ab82fdc71..395e4d3281 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -158,9 +158,10 @@ struct Nios2CPUClass {
 struct CPUNios2State {
     uint32_t regs[NUM_CORE_REGS];
 
-#if !defined(CONFIG_USER_ONLY)
+#ifdef CONFIG_USER_ONLY
+    int trap_code;
+#else
     Nios2MMU mmu;
-
     uint32_t irq_pending;
 #endif
 };
diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
index 9869083fa1..d76bdb2891 100644
--- a/linux-user/nios2/cpu_loop.c
+++ b/linux-user/nios2/cpu_loop.c
@@ -37,9 +37,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->trap_code) {
+            case 0:
                 qemu_log_mask(CPU_LOG_INT, "\nSyscall\n");
 
                 ret = do_syscall(env, env->regs[2],
@@ -53,26 +54,26 @@ 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;
+            default:
+                qemu_log_mask(CPU_LOG_INT, "\nTrap %d\n", env->trap_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..485b487665 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->trap_code.
+     */
+    R_TYPE(instr, code);
+    tcg_gen_st_i32(tcg_constant_i32(instr.imm5), cpu_env,
+                   offsetof(CPUNios2State, trap_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 */
-- 
2.25.1

[PATCH 2/2] linux-user/nios2: Map a real kuser page

[Richard Henderson]

The first word of page1 is data, so the whole thing
can't be implemented with emulation of addresses.

Hijack trap number 31 to implement cmpxchg.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c        | 35 +++++++++++++++++++++++++
 linux-user/nios2/cpu_loop.c | 52 ++++++++++++++++++++-----------------
 target/nios2/translate.c    |  9 -------
 3 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 01e9a833fb..e78c0af183 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1091,6 +1091,41 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMBState *env
 
 static void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
+    static const uint8_t kuser_page[128] = {
+        /* __kuser_helper_version */
+        [0x00] = 0x02, 0x00, 0x00, 0x00,
+
+        /* __kuser_cmpxchg */
+        [0x04] = 0xfa, 0x6f, 0x3b, 0x00,  /* trap 31 */
+                 0x3a, 0x28, 0x00, 0xf8,  /* ret */
+
+        /* __kuser_sigtramp */
+        [0x40] = 0xc4, 0x22, 0x80, 0x00,  /* movi r2, __NR_rt_sigreturn */
+                 0x3a, 0x68, 0x3b, 0x00,  /* trap 0 */
+    };
+
+    abi_ulong pg;
+    uint8_t *ph;
+
+    pg = target_mmap(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE,
+                     PROT_READ | PROT_WRITE,
+                     MAP_ANON | MAP_PRIVATE | MAP_FIXED, -1, 0);
+
+    /*
+     * If the mmap doesn't give us exactly page 1, there's nothing
+     * we can do, and it's unlikely that the program will be able
+     * to continue.  FIXME: Error out now?
+     */
+    if (pg == TARGET_PAGE_SIZE) {
+        ph = lock_user(VERIFY_WRITE, pg, sizeof(kuser_page), 0);
+        memcpy(ph, kuser_page, sizeof(kuser_page));
+        unlock_user(ph, pg, sizeof(kuser_page));
+        target_mprotect(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE,
+                        PROT_READ | PROT_EXEC);
+    } else {
+        target_munmap(pg, TARGET_PAGE_SIZE);
+    }
+
     regs->ea = infop->entry;
     regs->sp = infop->start_stack;
     regs->estatus = 0x3;
diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
index d76bdb2891..fd3f853ac2 100644
--- a/linux-user/nios2/cpu_loop.c
+++ b/linux-user/nios2/cpu_loop.c
@@ -20,11 +20,11 @@
 #include "qemu/osdep.h"
 #include "qemu.h"
 #include "cpu_loop-common.h"
+#include "signal-common.h"
 
 void cpu_loop(CPUNios2State *env)
 {
     CPUState *cs = env_cpu(env);
-    Nios2CPU *cpu = NIOS2_CPU(cs);
     target_siginfo_t info;
     int trapnr, ret;
 
@@ -71,6 +71,32 @@ void cpu_loop(CPUNios2State *env)
                 force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLTRP,
                                 env->regs[R_PC]);
                 break;
+
+            case 31: /* QEMU specific, for __kuser_cmpxchg */
+                {
+                    abi_ptr g = env->regs[4];
+                    uint32_t *h, n, o;
+
+                    if (g & 0x3) {
+                        force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, g);
+                        break;
+                    }
+                    ret = page_get_flags(g);
+                    if (!(ret & PAGE_VALID)) {
+                        force_sig_fault(TARGET_SIGSEGV, TARGET_SEGV_MAPERR, g);
+                        break;
+                    }
+                    if (!(ret & PAGE_READ) || !(ret & PAGE_WRITE)) {
+                        force_sig_fault(TARGET_SIGSEGV, TARGET_SEGV_ACCERR, g);
+                        break;
+                    }
+                    h = g2h(cs, g);
+                    o = env->regs[5];
+                    n = env->regs[6];
+                    env->regs[2] = qatomic_cmpxchg(h, o, n) - o;
+                    env->regs[R_PC] += 4;
+                }
+                break;
             }
             break;
 
@@ -81,29 +107,7 @@ void cpu_loop(CPUNios2State *env)
             queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
             break;
         case 0xaa:
-            switch (env->regs[R_PC]) {
-            /*case 0x1000:*/  /* TODO:__kuser_helper_version */
-            case 0x1004:      /* __kuser_cmpxchg */
-                start_exclusive();
-                if (env->regs[4] & 0x3) {
-                    goto kuser_fail;
-                }
-                ret = get_user_u32(env->regs[2], env->regs[4]);
-                if (ret) {
-                    end_exclusive();
-                    goto kuser_fail;
-                }
-                env->regs[2] -= env->regs[5];
-                if (env->regs[2] == 0) {
-                    put_user_u32(env->regs[6], env->regs[4]);
-                }
-                end_exclusive();
-                env->regs[R_PC] = env->regs[R_RA];
-                break;
-            /*case 0x1040:*/  /* TODO:__kuser_sigtramp */
-            default:
-                ;
-kuser_fail:
+            {
                 info.si_signo = TARGET_SIGSEGV;
                 info.si_errno = 0;
                 /* TODO: check env->error_code */
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 485b487665..77706dd805 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -795,15 +795,6 @@ static void nios2_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     dc->base.pc_next = pc + 4;
 
     /* Decode an instruction */
-
-#if defined(CONFIG_USER_ONLY)
-    /* FIXME: Is this needed ? */
-    if (pc >= 0x1000 && pc < 0x2000) {
-        t_gen_helper_raise_exception(dc, 0xaa);
-        return;
-    }
-#endif
-
     code = cpu_ldl_code(env, pc);
     op = get_opcode(code);
 
-- 
2.25.1

Re: [PATCH 0/2] linux-user/nios2: trap and kuser fixes

[Richard Henderson]

On 9/15/21 10:49 AM, Richard Henderson wrote:
> Based-on: 20210813131809.28655-1-peter.maydell@linaro.org
> ("linux-user: Clean up siginfo_t handling for arm, aarch64")
> ... for force_sig_fault().

Laurent, while I posted a follow-up to Peter's patch set, I'd prefer to go ahead and merge 
his first.  Just about everything I'm poking about with currently requires force_sig_fault().


r~

Re: [PATCH 0/2] linux-user/nios2: trap and kuser fixes

[Laurent Vivier]

Le 15/09/2021 à 19:54, Richard Henderson a écrit :
> On 9/15/21 10:49 AM, Richard Henderson wrote:
>> Based-on: 20210813131809.28655-1-peter.maydell@linaro.org
>> ("linux-user: Clean up siginfo_t handling for arm, aarch64")
>> ... for force_sig_fault().
> 
> Laurent, while I posted a follow-up to Peter's patch set, I'd prefer to go ahead and merge his
> first.  Just about everything I'm poking about with currently requires force_sig_fault().
> 

I'm going to send a PR with the "linux-user: split internals out of qemu.h" series, the Peter's
siginfo series will be in the next one.

Thanks,
Laurent