| Message ID | 20220527172731.1742837-2-shorne@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | OpenRISC Semihosting and Virt | expand |
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~
On Thu, Jun 02, 2022 at 08:39:21AM -0700, Richard Henderson wrote: > 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. That's right, I'll update it. > > +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 Sure, I did mention I kept these here for future (real) semihosting support. But I don't think that will happen. So I can remove. > > +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... OK. I'll look at this more. > > + default: > > + qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported " > > + "semihosting syscall %d\n", k); > > %u. OK. > > 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; > ... > } > } Yeah, that makes sense. I had written it in a way that would allow expanding for real semi-hosting. But I don't think we will do that with OpenRISC, so this is good enough. I am not sure if you saw the cover letter. I sent this RFC series to help illustrate two options for providing OpenRISC targets that support poweroff and reset. One option being using these NOP's, the second is to create a virt target with reset/poweroff hardware. I am kind of leaning towards dropping the semi-hosting patches and only moving forward with the virt patches. The reason being that 1. we would not need to expand the architecture spec to support the qemu virt platform, and we would need to document the NOP's formally, and 2. OpenRISC doesn't really support the full "semihosting" facilities for file open/close/write etc. Any thoughts? I guess this "semihosting" patch is pretty trivial. But, maybe it causes more confusion compared to just going with the virt route. Also, if we have virt I can't imagine anyone using the semihosting much. -Stafford
On 6/4/22 17:57, Stafford Horne wrote: > I am kind of leaning towards dropping the semi-hosting patches and only moving > forward with the virt patches. The reason being that 1. we would not need to > expand the architecture spec to support the qemu virt platform, and we would > need to document the NOP's formally, and 2. OpenRISC doesn't really support the > full "semihosting" facilities for file open/close/write etc. I agree that "virt" would to more for openrisc devel than these nops. > Also, if we have virt I can't imagine anyone using the semihosting much. IMO, semihosting is most valuable for writing regression tests and not much more. (You have no control over the exit status of qemu with normal shutdown, as compared with semihosting exit.) r~
diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak index 168101c39a..5b3ac89491 100644 --- a/configs/devices/or1k-softmmu/default.mak +++ b/configs/devices/or1k-softmmu/default.mak @@ -1,5 +1,7 @@ # Default configuration for or1k-softmmu +CONFIG_SEMIHOSTING=y + # Boards: # CONFIG_OR1K_SIM=y diff --git a/qemu-options.hx b/qemu-options.hx index b484640067..312c68b065 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4566,10 +4566,12 @@ ERST DEF("semihosting", 0, QEMU_OPTION_semihosting, "-semihosting semihosting mode\n", QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | - QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV) + QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV | + QEMU_ARCH_OPENRISC) SRST ``-semihosting`` - Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only). + Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V, + OpenRISC only). Note that this allows guest direct access to the host filesystem, so should only be used with a trusted guest OS. @@ -4581,11 +4583,12 @@ DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config, "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \ " semihosting configuration\n", QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | -QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV) +QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV | +QEMU_ARCH_OPENRISC) SRST ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]`` - Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V - only). + Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V, + OpenRISC only). Note that this allows guest direct access to the host filesystem, so should only be used with a trusted guest OS. @@ -4601,6 +4604,9 @@ SRST On RISC-V this implements the standard semihosting API, version 0.2. + On OpenRISC this only supports providing simulation exit and reset + facilities. + ``target=native|gdb|auto`` Defines where the semihosting calls will be addressed, to QEMU (``native``) or to GDB (``gdb``). The default is ``auto``, which diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h index b9584f10d4..4617f1272b 100644 --- a/target/openrisc/cpu.h +++ b/target/openrisc/cpu.h @@ -407,4 +407,6 @@ void cpu_set_fpcsr(CPUOpenRISCState *env, uint32_t val); #define CPU_INTERRUPT_TIMER CPU_INTERRUPT_TGT_INT_0 +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k); + #endif /* OPENRISC_CPU_H */ diff --git a/target/openrisc/helper.h b/target/openrisc/helper.h index d847814a28..2fe3e4e4ca 100644 --- a/target/openrisc/helper.h +++ b/target/openrisc/helper.h @@ -64,3 +64,4 @@ DEF_HELPER_FLAGS_1(rfe, 0, void, env) /* sys */ DEF_HELPER_FLAGS_3(mtspr, 0, void, env, tl, tl) DEF_HELPER_FLAGS_3(mfspr, TCG_CALL_NO_WG, tl, env, tl, tl) +DEF_HELPER_FLAGS_2(nop, 0, void, env, i32) diff --git a/target/openrisc/meson.build b/target/openrisc/meson.build index 84322086ec..1c1758b846 100644 --- a/target/openrisc/meson.build +++ b/target/openrisc/meson.build @@ -10,6 +10,7 @@ openrisc_ss.add(files( 'fpu_helper.c', 'gdbstub.c', 'interrupt_helper.c', + 'openrisc-semi.c', 'sys_helper.c', 'translate.c', )) diff --git a/target/openrisc/openrisc-semi.c b/target/openrisc/openrisc-semi.c new file mode 100644 index 0000000000..97d6aaacdb --- /dev/null +++ b/target/openrisc/openrisc-semi.c @@ -0,0 +1,54 @@ +/* + * OpenRISC Semihosting syscall interface. + * + * Copyright (c) 2022 Stafford Horne + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + */ + +#include "qemu/osdep.h" + +#include "cpu.h" +#include "exec/gdbstub.h" +#include "sysemu/runstate.h" +#include "qemu/log.h" + +#define HOSTED_EXIT 1 +#define HOSTED_RESET 13 + +static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret) +{ + cpu_set_gpr(env, 11, ret); +} + +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 + default: + qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported " + "semihosting syscall %d\n", k); + result = 0; + } + or1k_semi_return_u32(env, result); +} diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c index 48674231e7..eb698a527e 100644 --- a/target/openrisc/sys_helper.c +++ b/target/openrisc/sys_helper.c @@ -314,3 +314,8 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, /* for rd is passed in, if rd unchanged, just keep it back. */ return rd; } + +void HELPER(nop)(CPUOpenRISCState *env, target_ulong k) +{ + do_or1k_semihosting(env, k); +} diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c index 7b8ad43d5f..ec7b3b46ad 100644 --- a/target/openrisc/translate.c +++ b/target/openrisc/translate.c @@ -28,6 +28,7 @@ #include "qemu/qemu-print.h" #include "exec/cpu_ldst.h" #include "exec/translator.h" +#include "semihosting/semihost.h" #include "exec/helper-proto.h" #include "exec/helper-gen.h" @@ -780,6 +781,11 @@ static bool trans_l_sh(DisasContext *dc, arg_store *a) 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)); + } + return true; }
For OpenRISC we currently only use semihosting for system exit and reset. This patch implements that. The implementation uses a helper to delegate to the semihosting facility. The helper is marked as having side effects but currently does not have any. I have defined it like this as our other unimplemented semihosting calls will have side effects and return results in register r11. Signed-off-by: Stafford Horne <shorne@gmail.com> --- configs/devices/or1k-softmmu/default.mak | 2 + qemu-options.hx | 16 ++++--- target/openrisc/cpu.h | 2 + target/openrisc/helper.h | 1 + target/openrisc/meson.build | 1 + target/openrisc/openrisc-semi.c | 54 ++++++++++++++++++++++++ target/openrisc/sys_helper.c | 5 +++ target/openrisc/translate.c | 6 +++ 8 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 target/openrisc/openrisc-semi.c