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(-)

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;
-- 
2.9.3

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

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

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

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

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~

The OpenRISC architecture has the Power Management Register (PMR)
special purpose register to manage cpu power states.  The interesting
modes are:

 * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
 * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
 * Suspend Model (SUME) - Stop cpu and all units - wake on reset

The linux kernel will set DME when idle.

This patch implements the PMR SPR and halts the qemu cpu when there is a
change to DME or SME.  This means that openrisc qemu in no longer peggs
a host cpu at 100%.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
(Sorry, resending this again, there was something wrong with my mail setup
on the last)

Hello,

This patch seems work fine but I am not sure if it is the right way to do
trigger the halt signal.  Should I do it via raising an interrupt or
exception and exitting the cpu?

Also, I don't know if its due to this patch of an issue with the timer
interrupts.  After applying this patch the timer interrupts do not trigger
until a keypress is make.  i.e. something like this...

  $ sleep 5
  <hangs forever until a key is pressed>

It may or may not be related to this patch as I noticed sometime things
like this happened before this patch.


 target/openrisc/cpu.c        |  3 ++-
 target/openrisc/cpu.h        | 10 ++++++++++
 target/openrisc/interrupt.c  |  2 ++
 target/openrisc/machine.c    |  1 +
 target/openrisc/sys_helper.c | 12 ++++++++++++
 5 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index c9b3f22..1d6330c 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -51,7 +51,8 @@ static void openrisc_cpu_reset(CPUState *s)
     cpu->env.lock_addr = -1;
     s->exception_index = -1;
 
-    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP;
+    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP |
+                   UPR_PMP;
     cpu->env.dmmucfgr = (DMMUCFGR_NTW & (0 << 2)) | (DMMUCFGR_NTS & (6 << 2));
     cpu->env.immucfgr = (IMMUCFGR_NTW & (0 << 2)) | (IMMUCFGR_NTS & (6 << 2));
 
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index 938ccc3..2721432 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -140,6 +140,15 @@ enum {
     IMMUCFGR_HTR = (1 << 11),
 };
 
+/* Power management register */
+enum {
+    PMR_SDF = (15 << 0),
+    PMR_DME = (1 << 4),
+    PMR_SME = (1 << 5),
+    PMR_DCGE = (1 << 6),
+    PMR_SUME = (1 << 7),
+};
+
 /* Float point control status register */
 enum {
     FPCSR_FPEE = 1,
@@ -284,6 +293,7 @@ typedef struct CPUOpenRISCState {
     uint32_t immucfgr;        /* IMMU configure register */
     uint32_t esr;             /* Exception supervisor register */
     uint32_t evbar;           /* Exception vector base address register */
+    uint32_t pmr;             /* Power Management Register */
     uint32_t fpcsr;           /* Float register */
     float_status fp_status;
 
diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
index 2c91fab..3959671 100644
--- a/target/openrisc/interrupt.c
+++ b/target/openrisc/interrupt.c
@@ -60,6 +60,8 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
     env->sr |= SR_SM;
     env->sr &= ~SR_IEE;
     env->sr &= ~SR_TEE;
+    env->pmr &= ~PMR_DME;
+    env->pmr &= ~PMR_SME;
     env->tlb->cpu_openrisc_map_address_data = &cpu_openrisc_get_phys_nommu;
     env->tlb->cpu_openrisc_map_address_code = &cpu_openrisc_get_phys_nommu;
     env->lock_addr = -1;
diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c
index a82be62..a20cce7 100644
--- a/target/openrisc/machine.c
+++ b/target/openrisc/machine.c
@@ -138,6 +138,7 @@ static const VMStateDescription vmstate_env = {
         VMSTATE_UINT32(dmmucfgr, CPUOpenRISCState),
         VMSTATE_UINT32(immucfgr, CPUOpenRISCState),
         VMSTATE_UINT32(evbar, CPUOpenRISCState),
+        VMSTATE_UINT32(pmr, CPUOpenRISCState),
         VMSTATE_UINT32(esr, CPUOpenRISCState),
         VMSTATE_UINT32(fpcsr, CPUOpenRISCState),
         VMSTATE_UINT64(mac, CPUOpenRISCState),
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index fa3d6a4..cb1e085 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 "exception.h"
 
 #define TO_SPR(group, number) (((group) << 11) + (number))
 
@@ -141,6 +142,14 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
     case TO_SPR(5, 2):  /* MACHI */
         env->mac = deposit64(env->mac, 32, 32, rb);
         break;
+    case TO_SPR(8, 0):  /* PMR */
+        env->pmr = rb;
+        if (env->pmr & PMR_DME || env->pmr & PMR_SME) {
+            cpu_restore_state(cs, GETPC() + 4);
+            cs->halted = 1;
+            raise_exception(cpu, EXCP_HLT);
+        }
+        break;
     case TO_SPR(9, 0):  /* PICMR */
         env->picmr |= rb;
         break;
@@ -287,6 +296,9 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
         return env->mac >> 32;
         break;
 
+    case TO_SPR(8, 0):  /* PMR */
+        return env->pmr;
+
     case TO_SPR(9, 0):  /* PICMR */
         return env->picmr;
 
-- 
2.9.3

On 04/23/2017 11:54 PM, Stafford Horne wrote:
> The OpenRISC architecture has the Power Management Register (PMR)
> special purpose register to manage cpu power states.  The interesting
> modes are:
> 
>   * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
>   * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
>   * Suspend Model (SUME) - Stop cpu and all units - wake on reset
> 
> The linux kernel will set DME when idle.

And SUME would be, essentially, poweroff?  Perhaps at least for the purposes of 
QEMU; on real hardware one could press a button to assert reset and reboot.

> Also, I don't know if its due to this patch of an issue with the timer
> interrupts.  After applying this patch the timer interrupts do not trigger
> until a keypress is make.  i.e. something like this...
> 
>    $ sleep 5
>    <hangs forever until a key is pressed>
...
> +            cpu_restore_state(cs, GETPC() + 4);

This isn't correct.  You want

	cpu_restore_state(cs, GETPC());
	cs->env.pc += 4;

So what's happening is that you're re-executing the MTSPR and going back to 
sleep again.  Which probably explains the hang.


r~

On Tue, Apr 25, 2017 at 12:11:00PM +0200, Richard Henderson wrote:
> On 04/23/2017 11:54 PM, Stafford Horne wrote:
> > The OpenRISC architecture has the Power Management Register (PMR)
> > special purpose register to manage cpu power states.  The interesting
> > modes are:
> > 
> >   * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
> >   * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
> >   * Suspend Model (SUME) - Stop cpu and all units - wake on reset
> > 
> > The linux kernel will set DME when idle.
> 
> And SUME would be, essentially, poweroff?  Perhaps at least for the purposes
> of QEMU; on real hardware one could press a button to assert reset and
> reboot.

Yes, that is what I am thinking, but I could add this later, after some
reviews with other OpenRISC folks.

> > Also, I don't know if its due to this patch of an issue with the timer
> > interrupts.  After applying this patch the timer interrupts do not trigger
> > until a keypress is make.  i.e. something like this...
> > 
> >    $ sleep 5
> >    <hangs forever until a key is pressed>
> ...
> > +            cpu_restore_state(cs, GETPC() + 4);
> 
> This isn't correct.  You want
> 
> 	cpu_restore_state(cs, GETPC());
> 	cs->env.pc += 4;
> 
> So what's happening is that you're re-executing the MTSPR and going back to
> sleep again.  Which probably explains the hang.

I have changed to the above, but I think its essentially the same.  It
resumes after the MTSPR in both cases.

I fixed this now though, you should see another patch.  The issue is the
timer events get ignored once the cpu is in halt state, I added a
qemu_cpu_kick() call in the timer hardware to wake up the cpu on timer
interrupts.  Not sure if thats the best way to do it, but it works 100%
now.

-Stafford

On 04/25/2017 04:18 PM, Stafford Horne wrote:
> On Tue, Apr 25, 2017 at 12:11:00PM +0200, Richard Henderson wrote:
>> On 04/23/2017 11:54 PM, Stafford Horne wrote:
>>> The OpenRISC architecture has the Power Management Register (PMR)
>>> special purpose register to manage cpu power states.  The interesting
>>> modes are:
>>>
>>>    * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
>>>    * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
>>>    * Suspend Model (SUME) - Stop cpu and all units - wake on reset
>>>
>>> The linux kernel will set DME when idle.
>>
>> And SUME would be, essentially, poweroff?  Perhaps at least for the purposes
>> of QEMU; on real hardware one could press a button to assert reset and
>> reboot.
> 
> Yes, that is what I am thinking, but I could add this later, after some
> reviews with other OpenRISC folks.
> 
>>> Also, I don't know if its due to this patch of an issue with the timer
>>> interrupts.  After applying this patch the timer interrupts do not trigger
>>> until a keypress is make.  i.e. something like this...
>>>
>>>     $ sleep 5
>>>     <hangs forever until a key is pressed>
>> ...
>>> +            cpu_restore_state(cs, GETPC() + 4);
>>
>> This isn't correct.  You want
>>
>> 	cpu_restore_state(cs, GETPC());
>> 	cs->env.pc += 4;
>>
>> So what's happening is that you're re-executing the MTSPR and going back to
>> sleep again.  Which probably explains the hang.
> 
> I have changed to the above, but I think its essentially the same.  It
> resumes after the MTSPR in both cases.

It's not essentially the same.  GETPC is a host address.  Doing guest 
arithmetic on that is just wrong.

> I fixed this now though, you should see another patch.  The issue is the
> timer events get ignored once the cpu is in halt state, I added a
> qemu_cpu_kick() call in the timer hardware to wake up the cpu on timer
> interrupts.  Not sure if thats the best way to do it, but it works 100%
> now.

Ah, that could be.


r~

The OpenRISC architecture has the Power Management Register (PMR)
special purpose register to manage cpu power states.  The interesting
modes are:

 * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
 * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
 * Suspend Model (SUME) - Stop cpu and all units - wake on reset

The linux kernel will set DME when idle.

This patch implements the PMR SPR and halts the qemu cpu when there is a
change to DME or SME.  This means that openrisc qemu in no longer peggs
a host cpu at 100%.

In order for this to work we need to kick the CPU when timers are
expired.  Update the cpu timer to kick the cpu upon each timer event.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
Hello,

This patch seems work fine now.

Changes since v1:
  * Changed to kick cpu in timer
  * Increment pc separate from restore

 hw/openrisc/cputimer.c       |  1 +
 target/openrisc/cpu.c        |  3 ++-
 target/openrisc/cpu.h        | 10 ++++++++++
 target/openrisc/interrupt.c  |  2 ++
 target/openrisc/machine.c    |  1 +
 target/openrisc/sys_helper.c | 13 +++++++++++++
 6 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
index a98c799..febc469 100644
--- a/hw/openrisc/cputimer.c
+++ b/hw/openrisc/cputimer.c
@@ -61,6 +61,7 @@ void cpu_openrisc_timer_update(OpenRISCCPU *cpu)
     }
     next = now + (uint64_t)wait * TIMER_PERIOD;
     timer_mod(cpu->env.timer, next);
+    qemu_cpu_kick(CPU(cpu));
 }
 
 void cpu_openrisc_count_start(OpenRISCCPU *cpu)
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index c9b3f22..1d6330c 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -51,7 +51,8 @@ static void openrisc_cpu_reset(CPUState *s)
     cpu->env.lock_addr = -1;
     s->exception_index = -1;
 
-    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP;
+    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP |
+                   UPR_PMP;
     cpu->env.dmmucfgr = (DMMUCFGR_NTW & (0 << 2)) | (DMMUCFGR_NTS & (6 << 2));
     cpu->env.immucfgr = (IMMUCFGR_NTW & (0 << 2)) | (IMMUCFGR_NTS & (6 << 2));
 
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index 938ccc3..2721432 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -140,6 +140,15 @@ enum {
     IMMUCFGR_HTR = (1 << 11),
 };
 
+/* Power management register */
+enum {
+    PMR_SDF = (15 << 0),
+    PMR_DME = (1 << 4),
+    PMR_SME = (1 << 5),
+    PMR_DCGE = (1 << 6),
+    PMR_SUME = (1 << 7),
+};
+
 /* Float point control status register */
 enum {
     FPCSR_FPEE = 1,
@@ -284,6 +293,7 @@ typedef struct CPUOpenRISCState {
     uint32_t immucfgr;        /* IMMU configure register */
     uint32_t esr;             /* Exception supervisor register */
     uint32_t evbar;           /* Exception vector base address register */
+    uint32_t pmr;             /* Power Management Register */
     uint32_t fpcsr;           /* Float register */
     float_status fp_status;
 
diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
index 2c91fab..3959671 100644
--- a/target/openrisc/interrupt.c
+++ b/target/openrisc/interrupt.c
@@ -60,6 +60,8 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
     env->sr |= SR_SM;
     env->sr &= ~SR_IEE;
     env->sr &= ~SR_TEE;
+    env->pmr &= ~PMR_DME;
+    env->pmr &= ~PMR_SME;
     env->tlb->cpu_openrisc_map_address_data = &cpu_openrisc_get_phys_nommu;
     env->tlb->cpu_openrisc_map_address_code = &cpu_openrisc_get_phys_nommu;
     env->lock_addr = -1;
diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c
index a82be62..a20cce7 100644
--- a/target/openrisc/machine.c
+++ b/target/openrisc/machine.c
@@ -138,6 +138,7 @@ static const VMStateDescription vmstate_env = {
         VMSTATE_UINT32(dmmucfgr, CPUOpenRISCState),
         VMSTATE_UINT32(immucfgr, CPUOpenRISCState),
         VMSTATE_UINT32(evbar, CPUOpenRISCState),
+        VMSTATE_UINT32(pmr, CPUOpenRISCState),
         VMSTATE_UINT32(esr, CPUOpenRISCState),
         VMSTATE_UINT32(fpcsr, CPUOpenRISCState),
         VMSTATE_UINT64(mac, CPUOpenRISCState),
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index fa3d6a4..44acf0d 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 "exception.h"
 
 #define TO_SPR(group, number) (((group) << 11) + (number))
 
@@ -141,6 +142,15 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
     case TO_SPR(5, 2):  /* MACHI */
         env->mac = deposit64(env->mac, 32, 32, rb);
         break;
+    case TO_SPR(8, 0):  /* PMR */
+        env->pmr = rb;
+        if (env->pmr & PMR_DME || env->pmr & PMR_SME) {
+            cpu_restore_state(cs, GETPC());
+            env->pc += 4;
+            cs->halted = 1;
+            raise_exception(cpu, EXCP_HLT);
+        }
+        break;
     case TO_SPR(9, 0):  /* PICMR */
         env->picmr |= rb;
         break;
@@ -287,6 +297,9 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
         return env->mac >> 32;
         break;
 
+    case TO_SPR(8, 0):  /* PMR */
+        return env->pmr;
+
     case TO_SPR(9, 0):  /* PICMR */
         return env->picmr;
 
-- 
2.9.3