Re: [RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Richard Henderson]

On 5/27/22 10:27, Stafford Horne wrote:
+void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k);
...
+DEF_HELPER_FLAGS_2(nop, 0, void, env, i32)

Just call the helper "semihosting" and be done with it.
And the helper wants an ifdef for system mode.

@@ -10,6 +10,7 @@ openrisc_ss.add(files(
    'fpu_helper.c',
    'gdbstub.c',
    'interrupt_helper.c',
+  'openrisc-semi.c',
    'sys_helper.c',
    'translate.c',
  ))

You want to add the new file for system mode only.
Or, now that I think of it, conditional on CONFIG_SEMIHOSTING itself.

+static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret)
+{
+    cpu_set_gpr(env, 11, ret);
+}

Let's drop this until you actually use it.  This appears to be attempting to mirror other, 
more complete semihosting, but missing the third "error" argument.


+void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k)
+{
+    uint32_t result;
+
+    switch (k) {
+    case HOSTED_EXIT:
+        gdb_exit(cpu_get_gpr(env, 3));
+        exit(cpu_get_gpr(env, 3));
+    case HOSTED_RESET:
+#ifndef CONFIG_USER_ONLY
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+        return;
+#endif

Do you in fact want to exit to the main loop after asking for reset?
That's the only way that "no return value" makes sense to me...

+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported "
+                      "semihosting syscall %d\n", k);

%u.

  static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
  {
+    if (semihosting_enabled() &&
+        a->k != 0) {
+        gen_helper_nop(cpu_env, tcg_constant_i32(a->k));
+    }

Perhaps cleaner to move the semihosting dispatch switch here, instead of leaving it to the 
runtime?  The reason we have a runtime switch for other guests is that the semihosting 
syscall number is in a register.  This would then be

    if (semihosting_enabled()) {
        switch (a->k) {
        case 0:
            break; /* normal nop */
        case HOSTED_EXIT:
            gen_helper_semihost_exit(cpu_R(dc, 3));
            break;
        case HOSTED_RESET:
            gen_helper_semihost_reset();
            tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);

            dc->base.is_jmp = DISAS_EXIT;
            break;
        ...
        }
    }


r~