| Message ID | fb69c137317a365dcb549dfef1ecd2fbff48e92c.1492384862.git.shorne@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 04/16/2017 04:23 PM, Stafford Horne wrote: > In openrisc simulators we use hooks like 'l.nop 1' to cause the > simulator to exit. Implement that for qemu too. > > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > Signed-off-by: Stafford Horne <shorne@gmail.com> As I said the first time this was posted: This is horrible. If you want to do something like this, it needs to be buried under a special run mode like -semihosting. > case 0x01: /* l.nop */ > LOG_DIS("l.nop %d\n", I16); > + { > + TCGv_i32 arg = tcg_const_i32(I16); > + gen_helper_nop(arg); > + } You also really really must special-case l.nop 0 so that it doesn't generate a function call. Just think of all the extra calls you're adding for every delay slot that couldn't be filled. r~
On Tue, Apr 18, 2017 at 12:52:52AM -0700, Richard Henderson wrote: > On 04/16/2017 04:23 PM, Stafford Horne wrote: > > In openrisc simulators we use hooks like 'l.nop 1' to cause the > > simulator to exit. Implement that for qemu too. > > > > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > > Signed-off-by: Stafford Horne <shorne@gmail.com> > > As I said the first time this was posted: This is horrible. > > If you want to do something like this, it needs to be buried under a special > run mode like -semihosting. Understood, I will revise this. I didnt know this was posted before. > > case 0x01: /* l.nop */ > > LOG_DIS("l.nop %d\n", I16); > > + { > > + TCGv_i32 arg = tcg_const_i32(I16); > > + gen_helper_nop(arg); > > + } > > You also really really must special-case l.nop 0 so that it doesn't generate > a function call. Just think of all the extra calls you're adding for every > delay slot that couldn't be filled. Yeah, that makes sense. Ill add that for l.nop 0. -Stafford > > r~
On Tue, Apr 18, 2017 at 11:20:55PM +0900, Stafford Horne wrote: > On Tue, Apr 18, 2017 at 12:52:52AM -0700, Richard Henderson wrote: > > On 04/16/2017 04:23 PM, Stafford Horne wrote: > > > In openrisc simulators we use hooks like 'l.nop 1' to cause the > > > simulator to exit. Implement that for qemu too. > > > > > > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > > > Signed-off-by: Stafford Horne <shorne@gmail.com> > > > > As I said the first time this was posted: This is horrible. > > > > If you want to do something like this, it needs to be buried under a special > > run mode like -semihosting. > > Understood, I will revise this. I didnt know this was posted before. > > > > case 0x01: /* l.nop */ > > > LOG_DIS("l.nop %d\n", I16); > > > + { > > > + TCGv_i32 arg = tcg_const_i32(I16); > > > + gen_helper_nop(arg); > > > + } > > > > You also really really must special-case l.nop 0 so that it doesn't generate > > a function call. Just think of all the extra calls you're adding for every > > delay slot that couldn't be filled. > > Yeah, that makes sense. Ill add that for l.nop 0. FYI, I am going to drop this patch for now. I think Waldemar can apply this patch for the time being. I looked through the semihosting route and I don't think poking l.nop through there makes much sense since that looks mainly for syscalls. I also considered making another flag like `-or1k-hacks`, but I figured that wouldnt be appropriate. I discussed a bit on #qemu and Alexander Graf suggested to properly define shutdown semantics for openrisc. Some examples were shown from ppc, s390 and arm. s390x http://git.qemu.org/?p=qemu.git;a=blob;f=target/s390x/helper.c;hb=HEAD#l265 Detects the cpu is in WAIT state and shutsdown qemu. ppc - platform http://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/e500.c;hb=HEAD#l936 Registers hardware device mpc8xxx_gpio which handles shutdown via gpio I will have a thought about this, it will require some kernel changes. -Stafford
On 04/22/2017 12:09 PM, Stafford Horne wrote: > I discussed a bit on #qemu and Alexander Graf suggested to properly define > shutdown semantics for openrisc. Some examples were shown from ppc, s390 > and arm. Yes, properly defining this in the spec goes a long way toward fixing the underlying problem. It's probably worth thinking about a wait-state condition as well, so that qemu can avoid staying pegged at 100% host cpu for an idle guest cpu. > ppc - platform > http://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/e500.c;hb=HEAD#l936 > Registers hardware device mpc8xxx_gpio which handles shutdown via gpio That's one possibility. Another is to define an SPR. r~
Hey Stafford, On Mon, Apr 17, 2017 at 08:23:51AM +0900, Stafford Horne wrote: > In openrisc simulators we use hooks like 'l.nop 1' to cause the > simulator to exit. Implement that for qemu too. > > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > Signed-off-by: Stafford Horne <shorne@gmail.com> I'm curious as to why this never got merged. I noticed I'm entirely able to shutdown or to reboot (which is mostly what I care about) Linux from OpenRISC. It just hangs. Thanks, Jason
On Wed, 27 Apr 2022 at 18:46, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hey Stafford, > > On Mon, Apr 17, 2017 at 08:23:51AM +0900, Stafford Horne wrote: > > In openrisc simulators we use hooks like 'l.nop 1' to cause the > > simulator to exit. Implement that for qemu too. > > > > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > > Signed-off-by: Stafford Horne <shorne@gmail.com> > > I'm curious as to why this never got merged. I noticed I'm entirely able > to shutdown or to reboot (which is mostly what I care about) Linux from > OpenRISC. It just hangs. This kind of thing needs to be either: (1) we're modelling real hardware and that real hardware has a device or other mechanism guest code can prod to cause a power-off or reboot. Then we model that device, and guest code triggers a shutdown or reboot exactly as it would on the real hardware. (2) there is an architecturally defined ABI for simulators, debug stubs, etc, that includes various operations typically including an "exit the simulator" function. (Arm semihosting is an example of this.) In that case we can implement that functionality, guarded by and controlled by the appropriate command line options. (This is generally not as nice as option 1, because the guest code has to be compiled to have support for semihosting and also because turning it on is usually also giving implicit permission for the guest code to read and write arbitrary host files, etc.) Either way, undocumented random hacks aren't a good idea, which is why this wasn't merged. thanks -- PMM
On Wed, Apr 27, 2022 at 07:47:33PM +0100, Peter Maydell wrote: > On Wed, 27 Apr 2022 at 18:46, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > Hey Stafford, > > > > On Mon, Apr 17, 2017 at 08:23:51AM +0900, Stafford Horne wrote: > > > In openrisc simulators we use hooks like 'l.nop 1' to cause the > > > simulator to exit. Implement that for qemu too. > > > > > > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > > > Signed-off-by: Stafford Horne <shorne@gmail.com> > > > > I'm curious as to why this never got merged. I noticed I'm entirely able > > to shutdown or to reboot (which is mostly what I care about) Linux from > > OpenRISC. It just hangs. > > This kind of thing needs to be either: > (1) we're modelling real hardware and that real hardware has a > device or other mechanism guest code can prod to cause a power-off > or reboot. Then we model that device, and guest code triggers a > shutdown or reboot exactly as it would on the real hardware. > (2) there is an architecturally defined ABI for simulators, debug > stubs, etc, that includes various operations typically including > an "exit the simulator" function. (Arm semihosting is an example > of this.) In that case we can implement that functionality, > guarded by and controlled by the appropriate command line options. > (This is generally not as nice as option 1, because the guest code > has to be compiled to have support for semihosting and also because > turning it on is usually also giving implicit permission for the > guest code to read and write arbitrary host files, etc.) > > Either way, undocumented random hacks aren't a good idea, which > is why this wasn't merged. Yes, this is what was brought up before. At that time semihosting was mentioned and I tried to understand what it was but didn't really understand it as a general concept. Is this something arm specific? Since the qemu or1k-sim defines our "simulator", I suspect I could add a definition of our simulator ABI to the OpenRISC architecture specification. The simulation uses of l.nop N as ABI hooks is a de-facto standard for OpenRISC. From the way you describe this now I take it if we document this as a architecture simulation ABI the patch would be accepted. -Stafford
Hi Stafford, On Thu, Apr 28, 2022 at 06:48:27AM +0900, Stafford Horne wrote: > On Wed, Apr 27, 2022 at 07:47:33PM +0100, Peter Maydell wrote: > > On Wed, 27 Apr 2022 at 18:46, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > > > Hey Stafford, > > > > > > On Mon, Apr 17, 2017 at 08:23:51AM +0900, Stafford Horne wrote: > > > > In openrisc simulators we use hooks like 'l.nop 1' to cause the > > > > simulator to exit. Implement that for qemu too. > > > > > > > > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > > > > Signed-off-by: Stafford Horne <shorne@gmail.com> > > > > > > I'm curious as to why this never got merged. I noticed I'm entirely able > > > to shutdown or to reboot (which is mostly what I care about) Linux from > > > OpenRISC. It just hangs. > > > > This kind of thing needs to be either: > > (1) we're modelling real hardware and that real hardware has a > > device or other mechanism guest code can prod to cause a power-off > > or reboot. Then we model that device, and guest code triggers a > > shutdown or reboot exactly as it would on the real hardware. > > (2) there is an architecturally defined ABI for simulators, debug > > stubs, etc, that includes various operations typically including > > an "exit the simulator" function. (Arm semihosting is an example > > of this.) In that case we can implement that functionality, > > guarded by and controlled by the appropriate command line options. > > (This is generally not as nice as option 1, because the guest code > > has to be compiled to have support for semihosting and also because > > turning it on is usually also giving implicit permission for the > > guest code to read and write arbitrary host files, etc.) > > > > Either way, undocumented random hacks aren't a good idea, which > > is why this wasn't merged. > > Yes, this is what was brought up before. At that time semihosting was mentioned > and I tried to understand what it was but didn't really understand it as a general > concept. Is this something arm specific? > > Since the qemu or1k-sim defines our "simulator", I suspect I could add a > definition of our simulator ABI to the OpenRISC architecture specification. The > simulation uses of l.nop N as ABI hooks is a de-facto standard for OpenRISC. > From the way you describe this now I take it if we document this as a > architecture simulation ABI the patch would be accepted. If that's what it takes, then that'd make sense. By the way, would this also help the reboot case? That's `reboot(RB_AUTOBOOT);`, which does: machine_restart() -> do_kernel_restart() -> atomic_notifier_chain_register(&restart_handler_list, nb) -> ??? As far as I can tell, nothing is wired into the reboot case for OpenRISC. Is this something that could be fixed in the kernel without having to patch QEMU? If so, then I could effectively get shutdown for my CI with the -no-reboot option, which is what I'm already doing for a few platforms. Jason
On Wed, 27 Apr 2022 at 23:27, Stafford Horne <shorne@gmail.com> wrote: > Yes, this is what was brought up before. At that time semihosting was mentioned > and I tried to understand what it was but didn't really understand it as a general > concept. Is this something arm specific? QEMU uses "semihosting" for the general concept of a syscall-like ABI provided by the model that allows guest code written to use it to access some facilities as if it were a program running on the host rather than running on bare metal. (I think the term derives originally from the Arm term for this kind of functionality, but the concept is not Arm-specific.) Arm defines an ABI which looks basically like a set of syscalls: code sets up some registers and executes a specific SVC or HLT or other magic instruction, and the implementation is supposed to then act on that. You can do things like "open file", "read file", "exit", etc. https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst The idea is that simulators and also debug stubs or debuggers can implement this, and then bare-metal code can be written to use it, mainly for debugging and test case purposes. The risc-v folks decided they needed similar functionality, and that the easiest way to do this was to align with the Arm specification and document the risc-v specific bits: https://github.com/riscv/riscv-semihosting-spec Some other architectures have an equivalent thing but which isn't the same set of functions as the Arm version; eg the nios2 version is documented here: https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=libgloss/nios2/nios2-semi.txt;hb=HEAD > Since the qemu or1k-sim defines our "simulator", I suspect I could add a > definition of our simulator ABI to the OpenRISC architecture specification. The > simulation uses of l.nop N as ABI hooks is a de-facto standard for OpenRISC. > From the way you describe this now I take it if we document this as a > architecture simulation ABI the patch would be accepted. If it's something that (a) is documented officially somewhere and (b) everybody is using consistently (ie guest code such as GNU newlib, QEMU, other simulators, etc), then yes, that's OK. It sounds like you just need to write down the details of your de-facto standard to turn it into a de-jure one :-) We would want to guard it behind the existing semihosting command line option, rather than having it enabled all the time, but that part should be straightforward. -- PMM
Hey again, On Thu, Apr 28, 2022 at 02:04:29AM +0200, Jason A. Donenfeld wrote: > By the way, would this also help the reboot case? That's > `reboot(RB_AUTOBOOT);`, which does: > > machine_restart() -> > do_kernel_restart() -> > atomic_notifier_chain_register(&restart_handler_list, nb) -> > ??? > > As far as I can tell, nothing is wired into the reboot case for > OpenRISC. Is this something that could be fixed in the kernel without > having to patch QEMU? If so, then I could effectively get shutdown for > my CI with the -no-reboot option, which is what I'm already doing for a > few platforms. I added 13 for this: https://lists.librecores.org/pipermail/openrisc/2022-April/003884.html When you go add these nops to the specification, please remember to add one for reboot too. Then, once that kernel code is merged and the specification published, it'll be sensible to add shutdown and reboot support to QEMU, per Peter's description. Jason
On Thu, Apr 28, 2022 at 01:16:51PM +0200, Jason A. Donenfeld wrote: > Hey again, > > On Thu, Apr 28, 2022 at 02:04:29AM +0200, Jason A. Donenfeld wrote: > > By the way, would this also help the reboot case? That's > > `reboot(RB_AUTOBOOT);`, which does: > > > > machine_restart() -> > > do_kernel_restart() -> > > atomic_notifier_chain_register(&restart_handler_list, nb) -> > > ??? > > > > As far as I can tell, nothing is wired into the reboot case for > > OpenRISC. Is this something that could be fixed in the kernel without > > having to patch QEMU? If so, then I could effectively get shutdown for > > my CI with the -no-reboot option, which is what I'm already doing for a > > few platforms. > > I added 13 for this: https://lists.librecores.org/pipermail/openrisc/2022-April/003884.html > > When you go add these nops to the specification, please remember to add > one for reboot too. Then, once that kernel code is merged and the > specification published, it'll be sensible to add shutdown and reboot > support to QEMU, per Peter's description. This sounds fair. -Stafford
diff --git a/target/openrisc/helper.h b/target/openrisc/helper.h index 4fd1a6b..b7838f5 100644 --- a/target/openrisc/helper.h +++ b/target/openrisc/helper.h @@ -59,3 +59,4 @@ DEF_HELPER_FLAGS_1(rfe, 0, void, env) /* sys */ DEF_HELPER_FLAGS_4(mtspr, 0, void, env, tl, tl, tl) DEF_HELPER_FLAGS_4(mfspr, TCG_CALL_NO_WG, tl, env, tl, tl, tl) +DEF_HELPER_1(nop, void, i32) diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c index 60c3193..2eaff87 100644 --- a/target/openrisc/sys_helper.c +++ b/target/openrisc/sys_helper.c @@ -22,6 +22,7 @@ #include "cpu.h" #include "exec/exec-all.h" #include "exec/helper-proto.h" +#include "sysemu/sysemu.h" #define TO_SPR(group, number) (((group) << 11) + (number)) @@ -286,3 +287,19 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, /* for rd is passed in, if rd unchanged, just keep it back. */ return rd; } + +/* In openrisc simulators nop can be used to do intersting + * things like shut down the simulator */ +void HELPER(nop)(uint32_t arg) +{ +#ifndef CONFIG_USER_ONLY + switch (arg) { + case 0x001: /* NOP_EXIT */ + case 0x00c: /* NOP_EXIT_SILENT */ + qemu_system_shutdown_request(); + break; + default: + break; + } +#endif +} diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c index 7c4cbf2..74df245 100644 --- a/target/openrisc/translate.c +++ b/target/openrisc/translate.c @@ -755,8 +755,11 @@ static void dec_misc(DisasContext *dc, uint32_t insn) switch (op1) { case 0x01: /* l.nop */ LOG_DIS("l.nop %d\n", I16); + { + TCGv_i32 arg = tcg_const_i32(I16); + gen_helper_nop(arg); + } break; - default: gen_illegal_exception(dc); break;
In openrisc simulators we use hooks like 'l.nop 1' to cause the simulator to exit. Implement that for qemu too. Reported-by: Waldemar Brodkorb <wbx@openadk.org> Signed-off-by: Stafford Horne <shorne@gmail.com> --- target/openrisc/helper.h | 1 + target/openrisc/sys_helper.c | 17 +++++++++++++++++ target/openrisc/translate.c | 5 ++++- 3 files changed, 22 insertions(+), 1 deletion(-)