On Tue, 28 Jan 2020 at 23:34, Keith Packard via <qemu-devel@nongnu.org> wrote: > > Adapt the arm semihosting support code for RISCV. This implementation > is based on the standard for RISC-V semihosting as documented in > > https://riscv.org/specifications/ > > Signed-off-by: Keith Packard <kei...@keithp.com> > > ---
> + * ARM Semihosting is documented in: > + * Semihosting for AArch32 and AArch64 Release 2.0 > + * https://static.docs.arm.com/100863/0200/semihosting.pdf True but irrelevant. You need to refer to a proper risc-v specification for your semihosting. > + case TARGET_SYS_EXIT: > + case TARGET_SYS_EXIT_EXTENDED: > + if (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 0) { > + /* > + * The A64 version of SYS_EXIT takes a parameter block, > + * so the application-exit type can return a subcode which > + * is the exit status code from the application. > + * SYS_EXIT_EXTENDED is an a new-in-v2.0 optional function > + * which allows A32/T32 guests to also provide a status code. > + */ This code and comment describe Arm semihosting, where we made this bad decision about the API for 32-bit Arm, fixed it for 64-bit Arm and then put in an extension to add the more sensible API to 32-bit as a backwards-compatible new function. Nobody else should make this API choice from the start. What does RISC-V want to do here? This should be in your specification. > + GET_ARG(0); > + GET_ARG(1); > + > + if (arg0 == ADP_Stopped_ApplicationExit) { > + ret = arg1; > + } else { > + ret = 1; > + } > + } else { > + /* > + * The A32/T32 version of SYS_EXIT specifies only > + * Stopped_ApplicationExit as normal exit, but does not > + * allow the guest to specify the exit status code. > + * Everything else is considered an error. > + */ > + ret = (args == ADP_Stopped_ApplicationExit) ? 0 : 1; > + } > + gdb_exit(env, ret); > + exit(ret); > + case TARGET_SYS_SYNCCACHE: > + /* > + * Clean the D-cache and invalidate the I-cache for the specified > + * virtual address range. This is a nop for us since we don't > + * implement caches. This is only present on A64. > + */ > + if (sizeof(target_ulong) == 8) { > + return 0; > + } > + /* fall through -- invalid for A32/T32 */ Again, this is an Arm-ism, where the old A32 ABI doesn't have this function but the new A64 one does. What does RISC-V want to specify here? > + default: > + fprintf(stderr, "qemu: Unsupported SemiHosting SWI 0x%02x\n", nr); > + cpu_dump_state(cs, stderr, 0); > + abort(); This is repeating the Arm ABI mistake of allowing implementations to just crash and burn if they're handed a function call they don't recognize. Ideally it should be avoided in a new ABI. > + } > +} thanks -- PMM
