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

Richard Henderson
2022-06-02T15:39:29Z

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~